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

style: more linting and refactoring #464

Merged
merged 37 commits into from
Jun 12, 2022
Merged

style: more linting and refactoring #464

merged 37 commits into from
Jun 12, 2022

Conversation

ROpdebee
Copy link
Owner

@ROpdebee ROpdebee commented Jun 9, 2022

Okay this turned out to be much larger than I anticipated. Whoops.

Pretty much just installed some new plugins and fixed the issues I agreed with and disabled those that I disagree with or are otherwise useless.

My initial intention was to made some stylistic parts more consistent (like the empty line issues discussed in some other recent PR, forgot which), so I checked prettier. Not a big fan of its output and restrictive ruleset though, so I've decided against it for now. Still on the lookout for another formatter. Edit: Turns out ESLint could do this all along.

@ROpdebee ROpdebee added the style label Jun 9, 2022
@ROpdebee ROpdebee requested a review from kellnerd June 9, 2022 19:08
It was actually already enabled in the previous commits, but I was
trying to solve all issues one-by-one.
@ROpdebee
Copy link
Owner Author

ROpdebee commented Jun 9, 2022

/deploy-preview to ensure I didn't break anything

@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #464 (cddcb2b) into main (6c87f3f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #464   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           46        46           
  Lines         1016      1008    -8     
  Branches       170       168    -2     
=========================================
- Hits          1016      1008    -8     
Impacted Files Coverage Δ
src/lib/logging/logger.ts 100.00% <ø> (ø)
src/lib/util/xhr.ts 100.00% <ø> (ø)
src/lib/MB/EditNote.ts 100.00% <100.00%> (ø)
src/lib/MB/URLs.ts 100.00% <100.00%> (ø)
src/lib/logging/consoleSink.ts 100.00% <100.00%> (ø)
src/lib/util/blob.ts 100.00% <100.00%> (ø)
src/lib/util/domain_dispatch.ts 100.00% <100.00%> (ø)
src/mb_enhanced_cover_art_uploads/fetch.ts 100.00% <100.00%> (ø)
src/mb_enhanced_cover_art_uploads/form.ts 100.00% <100.00%> (ø)
src/mb_enhanced_cover_art_uploads/maximise.ts 100.00% <100.00%> (ø)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c87f3f...cddcb2b. Read the comment docs.

github-actions bot added a commit that referenced this pull request Jun 9, 2022
style: more linting and refactoring (#464)
@github-actions
Copy link

github-actions bot commented Jun 9, 2022

This PR changes 1 built userscript(s):

See all changes

@ROpdebee
Copy link
Owner Author

ROpdebee commented Jun 9, 2022

Manually did some E2E tests, mainly focusing on the parts with non-trivial changes, and everything seems to work properly, so the unit tests did their job well 🙂

All these `public`s are a bit more verbose, but hopefully this will
force me to put some more thought into the access modifiers.
We switched to the GUI sink quite a while ago, but never dropped the
old banner implementation apparently.
Copy link
Collaborator

@kellnerd kellnerd left a comment

Choose a reason for hiding this comment

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

The new rules look sensible to me and I did a quick test to ensure that they are also working in my setup (by manually reverting some of the changes).

@ROpdebee
Copy link
Owner Author

Thanks for the review @kellnerd. I'm going to hold off on merging these until caa dims is deployed though, otherwise I'd have to fix all these new warnings in that PR.

@ROpdebee
Copy link
Owner Author

Actually, scratch that. If we delay the merging of this PR, we'd have to relint every other future PR as well.

@ROpdebee ROpdebee merged commit e3d776f into main Jun 12, 2022
@ROpdebee ROpdebee deleted the more-linting branch June 12, 2022 09:19
github-actions bot added a commit that referenced this pull request Jun 12, 2022
style: more linting and refactoring (#464)
@github-actions
Copy link

🚀 Released 1 new userscript version(s):

  • mb_enhanced_cover_art_uploads 2022.6.12 in 692d850

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants