-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update style guide #21478
Update style guide #21478
Conversation
@hayata-suenaga what's the status here? Are we still going to move forward with this PR? |
@youssef-lr Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Co-authored-by: Marc Glasser <marc.aaron.glasser@gmail.com>
Reviewer Checklist
Screenshots/VideosWebMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
Actually, I think we can enforce this by adding an eslint rule:
This rule also supports the |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
@roryabraham should we add this eslint rule after the migration is over? or should be add the rule and run |
I think we could do it right away before the migration, then migrate from |
🚀 Deployed to staging by https://github.com/roryabraham in version: 1.3.39-0 🚀
|
@roryabraham
|
Close, I guess I would suggest:
Does that make sense? Also, I don't feel that strongly if you think something else might be better. |
ah I understand. I gonna create an issue to add the new ESLint rule and run |
🚀 Deployed to production by https://github.com/francoisl in version: 1.3.39-11 🚀
|
https://expensify.slack.com/archives/C01GTK53T8Q/p1687567839079469?thread_ts=1685727880.152119&cid=C01GTK53T8Q