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

replace eslint with biome #74

Merged
merged 6 commits into from
Feb 22, 2024
Merged

Conversation

cre8
Copy link
Contributor

@cre8 cre8 commented Feb 21, 2024

eslint is not designed to support monorepos, biome seems to work pretty well for this.

The job is not run via lerna but by bionme on the root level. This allows to follow the links printent in the console. It okay to lint every project independent if it has updated files since all projects should be compliant to linting rules.

Signed-off-by: Mirko Mollik <[email protected]>
@cre8
Copy link
Contributor Author

cre8 commented Feb 21, 2024

@lukasjhan @berendsliedrecht I have added the biome config, but haven't applied any format changes so we can check if we are fine with the rules.

@lukasjhan
Copy link
Member

Some of code line failed with the current rules. Do you want to fix that later?

biome.json Outdated Show resolved Hide resolved
@lukasjhan
Copy link
Member

linter works well, 30+ founds :D

@cre8
Copy link
Contributor Author

cre8 commented Feb 22, 2024

@lukasjhan the intention of my first commits was to get feedback for the defined rules ;)
E.g. some people don't like it if you use the ! or any type in the code, but forcing these rules can lead to bigger changes in the code

cre8 and others added 2 commits February 22, 2024 08:51
exclude coverage folder

Co-authored-by: Lukas.J.Han <[email protected]>
Signed-off-by: Mirko Mollik <[email protected]>
Copy link

codecov bot commented Feb 22, 2024

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@9652e75). Click here to learn what that means.

Files Patch % Lines
packages/broswer-crypto/src/crypto.ts 0.00% 2 Missing ⚠️
packages/core/src/index.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #74   +/-   ##
=======================================
  Coverage        ?   65.70%           
=======================================
  Files           ?       33           
  Lines           ?     2114           
  Branches        ?      233           
=======================================
  Hits            ?     1389           
  Misses          ?      706           
  Partials        ?       19           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lukasjhan
Copy link
Member

@lukasjhan the intention of my first commits was to get feedback for the defined rules ;) E.g. some people don't like it if you use the ! or any type in the code, but forcing these rules can lead to bigger changes in the code

! and any. hmm... In my opinion, the rules in this PR is okay. Kinds of reasonable rules.

"rules": {
"recommended": true,
"suspicious": {
"noExplicitAny": "off"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to get rid of all the any uses. I can get rid of this later, so we can just leave it like this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@berendsliedrecht I opened an issue for this #79 , feel free to work on this :)

Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Thanks for the really quick pick up! Really liking the look of the setup and how easy it seems :).

@lukasjhan lukasjhan merged commit cc9923b into openwallet-foundation-labs:main Feb 22, 2024
8 checks passed
@cre8 cre8 deleted the feat/lint branch February 22, 2024 10:32
cre8 added a commit to cre8/sd-jwt-js that referenced this pull request Mar 8, 2024
Signed-off-by: Mirko Mollik <[email protected]>
Signed-off-by: Mirko Mollik <[email protected]>
Co-authored-by: Mirko Mollik <[email protected]>
Co-authored-by: Lukas.J.Han <[email protected]>
Signed-off-by: Mirko Mollik <[email protected]>
cre8 added a commit that referenced this pull request Mar 8, 2024
Signed-off-by: Mirko Mollik <[email protected]>
Signed-off-by: Mirko Mollik <[email protected]>
Co-authored-by: Mirko Mollik <[email protected]>
Co-authored-by: Lukas.J.Han <[email protected]>
Signed-off-by: Mirko Mollik <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants