Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

fix: Eslint not running on lint staged #63

Merged

Conversation

joaopedrodcf
Copy link
Contributor

Description

This fixes the issue #51 by adding eslint as a garment task in tspackage.
This also adds eslint-prettier-plugin so we can just use eslint to run both prettier and eslint.
Finally also fixed the version of local_garment that was still on 0.13 and now is on 0.16 (Doenst fix the issue #62 because the bump will not be made I think)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have added tests to cover my changes
  • I have made corresponding changes to the documentation

Disclaimer

By sending us your contributions, you are agreeing that your contribution is made subject to the terms of our Contributor Ownership Statement

@joaopedrodcf
Copy link
Contributor Author

Just noticed that the snapshots are not updated after this change, gonna update them 👌

@joaopedrodcf joaopedrodcf force-pushed the fix/eslint-not-running-on-lint branch from 4ee67e6 to e173ad6 Compare July 25, 2021 15:09
@@ -16,7 +16,18 @@ Object {
"args": Array [
"
/test_path/test/index.js
1:13 error Insert \`;⏎\` prettier/prettier
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change in snapshot makes sense @beshanoe ? Seems like the unit test is using the root eslint because prettier/prettier is not on the fixtures, so anytime we change the rules of the eslint for garment project this snapshot may fail.

    handler,
    {},
    {
      cwd,
      files
    }
  );```

Is it related to this function in the tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can review this later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay thanks 🙏
Do you want me to create an issue about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be nice so that we don't forget it, thanks!

@luisvieiragmr luisvieiragmr merged commit 4b9c92f into Farfetch:master Aug 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants