unlock-protocol / unlock

Ʉnlock is a protocol for memberships built on a blockchain.
https://unlock-protocol.com
MIT License
842 stars 249 forks source link

Eslint cleanup #14564

Closed julien51 closed 1 month ago

julien51 commented 2 months ago

Now that we have upgraded to eslint, I noticed we are using different rules in many of our packages, as well as only applying eslint to some files and not all the files in some packages.

For example, we use yarn run eslint src/ instead of yarn run eslint . which would cover all the files. We should make that in all the packages/applications.

Many packages do also include their own rules and we should avoid that.

This 200 USDC bounty is open for anyone who "normalizes" our use of eslint to the max!

Myestery commented 2 months ago

I'll love to complete my earlier work here.

julien51 commented 2 months ago

I'll love to complete my earlier work here.

Awesome! Assiogned to you then!

Myestery commented 1 month ago

Hi @julien51 I have removed many useless rules across all packages in this PR only 2 packages now require special ignores and rules as at now

julien51 commented 1 month ago

Thanks a lot @Myestery ! Please post a wallet address for a USDC payment on Base or Optimism!

Myestery commented 1 month ago

0x70b6361545b12c81eb8c66d3fc4a4f5a03f64337 Thanks

julien51 commented 1 month ago

@Myestery Actually there is an issue! I am trying to run yarn run lint in the unlock-app folder and it just hangs! I will have to revert your PR.

Please make sure linting runs/works in every single folder!

Myestery commented 1 month ago

@Myestery Actually there is an issue! I am trying to run yarn run lint in the unlock-app folder and it just hangs! I will have to revert your PR.

Please make sure linting runs/works in every single folder!

thats weird cos it worked locally and on the actions too

Myestery commented 1 month ago

I'm having an issue replicating the hang locally ATM and I confirmed from this job https://github.com/unlock-protocol/unlock/actions/runs/10836347274/job/30069954743 that the linting was successful on the unlock-app directory. I have also run yarn lint on every other folder using eslint and they were all successful. I'll like to know more about the issue you faced if possible

julien51 commented 1 month ago

@Myestery can you please open anew PR with the changes?

Myestery commented 1 month ago

Hi @julien51 I hope the fix worked this time

Myestery commented 1 month ago

Hello @julien51 , Little reminder for bounty payment on this issue

Myestery commented 1 month ago

Hello @julien51 , Little reminder for bounty payment on this issue

Hi @julien51, I don't know if you saw this

julien51 commented 1 month ago

@Myestery I can confirm we're still having issues with liunt that's hanging for some reason. I confirmed this with @0xTxbi for whom this is happening as well. I am not sure how to reproduce but please can you try on your end as well?

Myestery commented 1 month ago

@Myestery I can confirm we're still having issues with liunt that's hanging for some reason. I confirmed this with @0xTxbi for whom this is happening as well. I am not sure how to reproduce but please can you try on your end as well?

I could not replicate the issues at all. But have you tried yarn install again?

Myestery commented 1 month ago

I suspect its still using the old eslint version and not reading config from the config file thereby scanning node_modules as well. yarn install should fix this

julien51 commented 1 month ago

I suspect its still using the old eslint version and not reading config from the config file thereby scanning node_modules as well. yarn install should fix this

I don't think that is what is happening. I did a full cleanup of the repo locally (git clean -fdx) and then I did yarn && yarn build. At that point it looks like the lit command succeeds... but later when i do changes in the unlock-app app, eventually the lint command fails.

Myestery commented 1 month ago

trying again...

Myestery commented 1 month ago

I was finally able to replicate this issue after I ran yarn build, I had to add an ignore rule to the .next folder that nextjs adds. The fix is here https://github.com/unlock-protocol/unlock/pull/14633

julien51 commented 1 month ago

I was finally able to replicate this issue after I ran yarn build, I had to add an ignore rule to the .next folder that nextjs adds. The fix is here https://github.com/unlock-protocol/unlock/pull/14633

Awesome! Please let me test things tomorrow and pay the bounty after that! Thank you so much !

Myestery commented 1 month ago

Hello, were you able to test it yet?

julien51 commented 1 month ago

yes! I also just paid the bounty! https://basescan.org/tx/0xc235f2be4a1fcfed979ef7c88909927345a70c6cfbef64ce0af4dcf3909d96e0

Thanks for your hard work!

Myestery commented 1 month ago

Many thanks looking foward to doing more