Skip to content
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-11-11] [$250] Automate author checklist with ESLint rules #50154

Closed
szymonrybczak opened this issue Oct 3, 2024 · 26 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@szymonrybczak
Copy link
Contributor

szymonrybczak commented Oct 3, 2024

Problem:

The current Author Checklist for creating a Pull Request contains 48 checks, which is an excessive number of items to be manually verified every time a PR is submitted. This lengthy process can slow down the development workflow.

Solution

Automate checks with ESLint rules where possible, as we've found some checklist items that can be easily automated.

  • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.

    Here’s a diff presenting a potential implementation: callstack-internal/eslint-config-expensify@b31ad44.

    After applying diff locally there’s only one error in whole codebase:

    ❯ npm run lint
    
    > new.expensify@9.0.37-0 lint
    > NODE_OPTIONS=--max_old_space_size=8192 eslint . --max-warnings=0 --cache --cache-location=node_modules/.cache/eslint
    
    This is not a desktop build, adding babel-plugin-annotate-react
    babel.config.js
      - api.version: 7.24.5
      - api.env: development
      - process.env.NODE_ENV: undefined
      - process.env.BABEL_ENV: undefined
      - running in:  undefined
    
    ~/Users/szymonrybczak/callstack/App/src/pages/workspace/distanceRates/PolicyDistanceRatesSettingsPage.tsx
      102:30  error  The left side of conditional rendering should be a boolean, not Unit  rulesdir/boolean-conditional-rendering
    
    ✖ 1 problem (1 error, 0 warnings)
    

    The lint rule examines the left-side condition of an expression. If it detects a non-boolean value, it raises an error — once this rule is implemented, we can remove the corresponding check from the Author Checklist in the Pull Request template.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021849137380340533101
  • Upwork Job ID: 1849137380340533101
  • Last Price Increase: 2024-10-23
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @sakluger
@szymonrybczak szymonrybczak added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Oct 3, 2024
Copy link

melvin-bot bot commented Oct 3, 2024

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@mountiny
Copy link
Contributor

mountiny commented Oct 3, 2024

@rayane-djouah for implementation, what is your eta for the pr, soˇe diff is already available

@rayane-djouah
Copy link
Contributor

I'm starting right now

@rayane-djouah
Copy link
Contributor

I've encountered issues with the isBoolean function and its implementation in the OP's code diff. Additionally, I've noticed problems with the formatting of error messages.
Also, I have added tests for this rule.

Here's the draft PR: Expensify/eslint-config-expensify#121. Note: I am still investigating a lint error.

@rayane-djouah
Copy link
Contributor

Fixed the Lint errors 🎉

@rayane-djouah
Copy link
Contributor

@szymonrybczak @mountiny PR ready for review

@melvin-bot melvin-bot bot added the Overdue label Oct 7, 2024
@mountiny
Copy link
Contributor

mountiny commented Oct 7, 2024

@szymonrybczak is going to review the eslint PR

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 7, 2024
@sakluger sakluger added the Reviewing Has a PR in review label Oct 9, 2024
@melvin-bot melvin-bot bot removed the Overdue label Oct 9, 2024
@muttmuure muttmuure moved this from LOW to HIGH in [#whatsnext] #quality Oct 15, 2024
@mountiny
Copy link
Contributor

@rayane-djouah merged, can you please make App PR now 🙏

@rayane-djouah
Copy link
Contributor

rayane-djouah commented Oct 18, 2024

I'm waiting on #50935 to be merged

@mountiny
Copy link
Contributor

Merged

@sakluger
Copy link
Contributor

@mountiny it looks like this one is pretty much complete! How should we handle payment for this issue? $250 to @rayane-djouah in 7 days?

@mountiny
Copy link
Contributor

Yes!

@sakluger sakluger changed the title Automate author checklist with ESLint rules [$250] Automate author checklist with ESLint rules Oct 23, 2024
Copy link

melvin-bot bot commented Oct 23, 2024

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

@sakluger sakluger added Internal Requires API changes or must be handled by Expensify staff External Added to denote the issue can be worked on by a contributor and removed Internal Requires API changes or must be handled by Expensify staff labels Oct 23, 2024
Copy link

melvin-bot bot commented Oct 23, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021849137380340533101

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 23, 2024
Copy link

melvin-bot bot commented Oct 23, 2024

Current assignee @rayane-djouah is eligible for the External assigner, not assigning anyone new.

@sakluger
Copy link
Contributor

Summarizing payment on this issue:

Contributor: @rayane-djouah $250, sent offer via Upwork: https://www.upwork.com/nx/wm/offer/104560122

@mountiny
Copy link
Contributor

@rayane-djouah we still need to update App right?

@rayane-djouah
Copy link
Contributor

Yes, I'm working on an App PR

@sakluger
Copy link
Contributor

Okay we'll wait to pay out until 7 days after the App PR is deployed.

@rayane-djouah
Copy link
Contributor

@mountiny @szymonrybczak App PR ready for review: #51741

@rayane-djouah
Copy link
Contributor

Merged

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 4, 2024
@melvin-bot melvin-bot bot changed the title [$250] Automate author checklist with ESLint rules [HOLD for payment 2024-11-11] [$250] Automate author checklist with ESLint rules Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 4, 2024
Copy link

melvin-bot bot commented Nov 4, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.56-9 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-11-11. 🎊

For reference, here are some details about the assignees on this issue:

Copy link

melvin-bot bot commented Nov 4, 2024

@rayane-djouah @sakluger The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@rayane-djouah
Copy link
Contributor

No BugZero Checklist is required as this is a new feature. Additionally, no regression testing is necessary because this is a tool for developers.

Copy link

melvin-bot bot commented Nov 11, 2024

Payment Summary

Upwork Job

BugZero Checklist (@sakluger)

  • I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
  • I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1849137380340533101/hired)
  • I have paid out the Upwork contracts or cancelled the ones that are incorrect
  • I have verified the payment summary above is correct

@github-project-automation github-project-automation bot moved this from HIGH to Done in [#whatsnext] #quality Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
Development

No branches or pull requests

4 participants