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.

Sign up for GitHub

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

Jump to bottom

chore: remove prettier, and default to eslint #1495

Merged
merged 15 commits into from
Sep 4, 2023
Merged

chore: remove prettier, and default to eslint #1495

merged 15 commits into from
Sep 4, 2023

Conversation

danisharora099
Copy link
Collaborator

@danisharora099 danisharora099 commented Aug 22, 2023 •
edited
Loading

This PR

  • removes prettier altogether
  • uses eslint for formatting + linting
  • sets vscode default formatter to eslint, from prettier
  • reruns npm run proto
    • since we don't run prettier on it anymore, there is a huge diff
    • ideally, we should remove proto from eslintIgnore but i am not sure why it was added to ignore (704f277)

@github-actions
Copy link

github-actions bot commented Aug 22, 2023 •
edited
Loading

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 28.67 KB (+0.02% 🔺) 574 ms (+0.02% 🔺) 243 ms (-41.8% 🔽) 816 ms
Waku Simple Light Node 298.5 KB (0%) 6 s (0%) 1.2 s (+27.89% 🔺) 7.1 s
ECIES encryption 28.68 KB (-0.01% 🔽) 574 ms (-0.01% 🔽) 241 ms (-52.38% 🔽) 815 ms
Symmetric encryption 28.68 KB (-0.01% 🔽) 574 ms (-0.01% 🔽) 388 ms (+47.79% 🔺) 962 ms
DNS discovery 118.6 KB (0%) 2.4 s (0%) 625 ms (+20.32% 🔺) 3 s
Privacy preserving protocols 122.62 KB (0%) 2.5 s (0%) 664 ms (+25.65% 🔺) 3.2 s
Light protocols 28.68 KB (+0.02% 🔺) 574 ms (+0.02% 🔺) 299 ms (-16.92% 🔽) 872 ms
History retrieval protocols 27.92 KB (+0.03% 🔺) 559 ms (+0.03% 🔺) 285 ms (-28.93% 🔽) 843 ms
Deterministic Message Hashing 5.64 KB (0%) 113 ms (0%) 187 ms (+68.36% 🔺) 300 ms

@danisharora099 danisharora099 changed the title chore: rm unwanted prettierrc/eslint config chore: remove prettier, and default to eslint Aug 22, 2023
@danisharora099 danisharora099 marked this pull request as ready for review August 22, 2023 10:21
@danisharora099 danisharora099 requested a review from a team as a code owner August 22, 2023 10:21
fryorcraken
fryorcraken requested changes Aug 22, 2023
View reviewed changes
Copy link
Collaborator

@fryorcraken fryorcraken left a comment

Choose a reason for hiding this comment

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

Do not run format or lint on generated files:

  • it just creates more mess as you know have 2 scripts that breaks each-other outputs (same as you had with eslint and prettier)
  • the files are auto-generated so nobody is going to "mess up" the formatting
  • the files are auto-generated so even if the linter find an issue, there is nothing you will be able to do to fix it.

@danisharora099 danisharora099 mentioned this pull request Aug 28, 2023
Closed
24 tasks
@danisharora099
Copy link
Collaborator Author

danisharora099 commented Aug 28, 2023 •
edited
Loading

@fryorcraken
looks like prettified code is committed to master, that is different from the raw output. thus, there is a diff with the committed code which leads to failing proto check. https://github.com/waku-org/js-waku/actions/runs/5997113882/job/16263132678?pr=1495
but i guess merging this PR with the raw autogen code to master, would then not result in failing proto checks as it would make the non-prettified code/raw autogen code as default?

--
i reran npm run proto and committed it to this PR without linting it, thus keeping it raw.

fryorcraken
fryorcraken reviewed Aug 29, 2023
View reviewed changes
.eslintrc.json Outdated Show resolved Hide resolved
.eslintrc.json Outdated Show resolved Hide resolved
@danisharora099 danisharora099 requested review from a team and fryorcraken August 31, 2023 13:52
weboko
weboko reviewed Aug 31, 2023
View reviewed changes
.eslintrc.json Outdated Show resolved Hide resolved
weboko
weboko reviewed Aug 31, 2023
View reviewed changes
packages/proto/src/lib/filter.ts
w.uint32(10);
w.string(obj.contentTopic);
}
if ((obj.contentTopic != null && obj.contentTopic !== '')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh no, why do we change to single quote?

Copy link
Collaborator Author

@danisharora099 danisharora099 Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

we don't -- this is just the proto autogen files that we don't run linting on anymore :)

@danisharora099 danisharora099 merged commit 78f64f6 into master Sep 4, 2023
9 of 10 checks passed
@danisharora099 danisharora099 deleted the chore/revert-prettierrc branch September 4, 2023 07:07
@danisharora099 danisharora099 mentioned this pull request Sep 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fryorcraken fryorcraken fryorcraken approved these changes

@weboko weboko Awaiting requested review from weboko

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danisharora099 @fryorcraken @weboko