Skip to content

Run linter with prettier 2 #1477

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

Merged
merged 4 commits into from
Sep 29, 2020
Merged

Run linter with prettier 2 #1477

merged 4 commits into from
Sep 29, 2020

Conversation

alubbe
Copy link
Member

@alubbe alubbe commented Sep 28, 2020

Summary

Some files haven't had the linter applied to, so let's fix that :)

Test plan

No new tests are needed

@alubbe alubbe requested review from guyonroche, Alanscut and Siemienik and removed request for guyonroche September 28, 2020 10:37
Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

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

This pr again introduce lots of changes, which has no sense. Lets we keep current style rules

@alubbe
Copy link
Member Author

alubbe commented Sep 28, 2020

Agreed - I've reset the arrow-parens behaviour back to prettier v1 ('as-needed'). The other changes were always 'wrong', I think and they looks safe enough to merge

Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

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

I've added more comments, If is it fine to you I'm going to propose some rules changes according to my comments 😉

Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

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

After some investigation, I changed max-len option to 100. Before was 160 - that value is too much for properly priettier 2 working.

As effect some improvements was introduced like:

https://github.com/exceljs/exceljs/pull/1477/files#diff-a083b92106d7d843e5f325cb350dbf25L102-R109

image

@alubbe
Copy link
Member Author

alubbe commented Sep 29, 2020

Looks good, I also found another typo - fixed, will merge

@alubbe alubbe merged commit d0e57f0 into exceljs:master Sep 29, 2020
@alubbe alubbe deleted the linter branch September 29, 2020 07:27
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.

2 participants