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 2023-10-02] [$1000] Create a lint rule for unused styles and translation keys #22212

Closed
luacmartins opened this issue Jul 4, 2023 · 72 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.

Comments

@luacmartins
Copy link
Contributor

luacmartins commented Jul 4, 2023

Problem

We often add new styles to styles.js or copy to en.js and es.js, but when we remove the last instance using them in App, sometimes we forget to also remove their definition from en.js, es.js or styles.js.

Why is this important

Keeps the codebase clean

Solution

Add a lint rule to eslint-config-expensify to catch unused styles and translation keys so we can remove them appropriately.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0140cfa1480a9e4861
  • Upwork Job ID: 1676651464070823936
  • Last Price Increase: 2023-07-05
@luacmartins luacmartins added Daily KSv2 NewFeature Something to build that is a new item. labels Jul 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 4, 2023

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jul 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 4, 2023

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

@luacmartins luacmartins self-assigned this Jul 4, 2023
@luacmartins
Copy link
Contributor Author

No design changes here. Unassigning @shawnborton

@luacmartins luacmartins changed the title Create a lint rule for unused styles and copy keys Create a lint rule for unused styles and translation keys Jul 4, 2023
@gedu
Copy link
Contributor

gedu commented Jul 5, 2023

Hey, I am Edu from Callstack - an expert contributor group.

I can work on this one

@luacmartins luacmartins added the External Added to denote the issue can be worked on by a contributor label Jul 5, 2023
@melvin-bot melvin-bot bot changed the title Create a lint rule for unused styles and translation keys [$1000] Create a lint rule for unused styles and translation keys Jul 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2023

Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 5, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 5, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

@luacmartins luacmartins removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 5, 2023
@gedu
Copy link
Contributor

gedu commented Jul 6, 2023

Working on handling deep storage per key, and searching

@gedu
Copy link
Contributor

gedu commented Jul 6, 2023

Working on reading the styles and translation files, and adding tests compatible with JSX

@gedu
Copy link
Contributor

gedu commented Jul 7, 2023

Need to work on another issue, couldn't work on it today

@melvin-bot melvin-bot bot added the Overdue label Jul 10, 2023
@NicMendonca
Copy link
Contributor

@gedu any update here?

@melvin-bot melvin-bot bot removed the Overdue label Jul 10, 2023
@luacmartins
Copy link
Contributor Author

That's awesome! Great work!

@melvin-bot melvin-bot bot added the Overdue label Sep 15, 2023
@luacmartins
Copy link
Contributor Author

@gedu is still working on the PR

@melvin-bot melvin-bot bot removed the Overdue label Sep 15, 2023
@gedu
Copy link
Contributor

gedu commented Sep 15, 2023

I'm currently resolving the conflicts, and I've added some comments to the PR. However, due to the new style format that incorporates a Theme object, I need to modify the way we retrieve and integrate these styles within the codebase.

@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@luacmartins
Copy link
Contributor Author

PR is still being worked on as per @gedu's update above

@melvin-bot melvin-bot bot removed the Overdue label Sep 18, 2023
@gedu
Copy link
Contributor

gedu commented Sep 18, 2023

@luacmartins PR updated, also found another unused style because a component was removed, so it is working great

@luacmartins
Copy link
Contributor Author

Awesome! Is the PR ready for review?

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 19, 2023
@gedu
Copy link
Contributor

gedu commented Sep 19, 2023

@luacmartins Yes it is 🙌

@gedu
Copy link
Contributor

gedu commented Sep 21, 2023

@luacmartins Comments fixed and conflicts solved

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Sep 25, 2023
@melvin-bot melvin-bot bot changed the title [$1000] Create a lint rule for unused styles and translation keys [HOLD for payment 2023-10-02] [$1000] Create a lint rule for unused styles and translation keys Sep 25, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.73-1 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 2023-10-02. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

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

  • @gedu does not require payment (Contractor)
  • @fedirjh requires payment

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Oct 1, 2023
@NicMendonca
Copy link
Contributor

image

@melvin-bot melvin-bot bot removed the Overdue label Oct 3, 2023
@luacmartins
Copy link
Contributor Author

@NicMendonca correct, @fedirjh did not review that PR.

@NicMendonca
Copy link
Contributor

ok cool, we're all set here then!

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 Daily KSv2 External Added to denote the issue can be worked on by a contributor NewFeature Something to build that is a new item.
Projects
None yet
Development

No branches or pull requests

6 participants