-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] [TS Migration] Standartize between Boolean or !! #39126
Comments
ProposalPlease re-state the problem that we are trying to solve in this issue.Standardise between Boolean or !!. What is the root cause of that problem?New check using ESlint. What changes do you think we should make in order to solve the problem?Slack discussion: https://expensify.enterprise.slack.com/archives/C01GTK53T8Q/p1713329747825759. Looks like we want to go ahead with
|
10 days overdue. Is anyone even seeing these? Hello? |
@mountiny Can we pick this since TS migration is almost done? I've sent a proposal above. |
@ShridharGoel Not yet, let's wait until migration is fully finished |
12 days overdue. Walking. Toward. The. Light... |
This is on hold Melvin.. |
Details: We want to standardize with just one approach, !! and Boolean() are almost identical and we want to ensure just one is used in the codebase to avoid cases like this 😅 offline = Boolean(network.shouldForceOffline) || !!network.isOffline; This includes writing a proposal on Slack, adjusting the code in the repository and perhaps adding an eslint rule that ensures only one approach is used. I myself believe !! is more readable more concise and faster in runtime. This issue requires a minimal understanding of how Expensify process works, before coding the contributor have to post a Problem/Solution on open-source channel |
@blazejkustra Do we need to post on Slack? We generally post proposals on GitHub itself, I've posted a proposal above. |
The issue is still on hold, please wait until it's open for external contributors @ShridharGoel |
This issue has not been updated in over 14 days. eroding to Weekly issue. |
Job added to Upwork: https://www.upwork.com/jobs/~01111a2979f5f2ac8c |
Triggered auto assignment to @CortneyOfstad ( |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary ( |
PR is being actively reviewed so no additional update at this stage! |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-06-18. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-06-20. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@ishpaul777 can you confirm if this needs a regression test please before EOD to help avoid a delay in payment? Thank you! |
Bump @ishpaul777 ^^^ |
Sorry i missed the ping. We dont need a regression test, this issue was not about adding a new functionality but to standardize on a code pattern. |
@ShridharGoel I had to reinvite you to the job post because for some reason you were not listed in Upwork — let me know once you accept and I can get that paid ASAP! |
not overdue, @ShridharGoel gentle bump on ^, so we can close this |
@CortneyOfstad, @ShridharGoel, @aldo-expensify, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@ShridharGoel let me know once you accept and I can get this paid ASAP. Thanks! |
Accepted, thanks. |
Payment Summary@ishpaul777 — paid $250 via Upwork |
Follow up issue for TS migration project. Coming from this spreadsheet
Lets pick the one which can be enforced with eslint
Any more details for this one cc @blazejkustra @fabioh8010
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @CortneyOfstadThe text was updated successfully, but these errors were encountered: