-
Notifications
You must be signed in to change notification settings - Fork 28
chore(fix): introduce prettier linter #139
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
chore(fix): introduce prettier linter #139
Conversation
|
Introducing and applying prettier linter. @jbonofre please check when you have time 👋 |
jbonofre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks !
As lint is part of the Console CI, it makes sense to help contributors with lint process 😄
|
@binarycat0 the CI is failing: The script has to be fixed. |
jbonofre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the CI.
|
@jbonofre fix is merged. Should work well now. Need to rerun the CI check 🙏 |
snazy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatting changes are just formatting changes.
Cannot judge on the eslint/prettier settings, and I suspect that's largely going into personal preferences & opinions, so I'm okay with the style.
Would just be good if someone else could take a look into the config and build files.
Thanks for review. I agree that preferences are personal, and would be glad to get a comment from someone more experienced FE dev, because neither am I a professional FE dev 😄 |
|
I'm fine with the new linter, but I do not work on UI code at all 😅 @adam-christian-software @vignesh-manel @developerzohaib786 @cccs-cat001 : WDYT? |
|
So, if there are no strong concerns, I think the current settings might be accepted and then corrected if needed. |
adam-christian-software
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! This will be wonderful in terms of standardization.
|
I'm very much a fan of having a technically defined and enforced code style. As it's a "broad change", let's give people maybe one more day and then get this change merged, unless someone has strong technical reasons. We can always adjust & tune things later. |
cccs-cat001
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a reasonable improvement to me! I'm not much of a UI guy though. I'll trust that all the changes are just cosmetics ;)
fix: format/lint