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

Jump to bottom

Fix to follow the brace-style ESLint rule #1312

Closed
wants to merge 13 commits into from
Closed

Fix to follow the brace-style ESLint rule #1312

wants to merge 13 commits into from

Conversation

voloshinskii
Copy link
Contributor

@voloshinskii voloshinskii commented Dec 8, 2019 •
edited

Fixes to follow the brace-style rule (https://eslint.org/docs/rules/brace-style)

reosarevok
reosarevok requested changes Dec 9, 2019
View reviewed changes
Copy link
Member

@reosarevok reosarevok left a comment

Choose a reason for hiding this comment

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

Few small issues here, it does seem mostly fine though :)

root/static/scripts/relationship-editor/common/dialog.js Outdated Show resolved Hide resolved
root/static/scripts/relationship-editor/common/multiselect.js Outdated Show resolved Hide resolved
root/static/scripts/relationship-editor/generic.js Outdated Show resolved Hide resolved
root/static/scripts/relationship-editor/generic.js Outdated Show resolved Hide resolved
root/static/scripts/relationship-editor/generic.js Outdated Show resolved Hide resolved
root/static/scripts/relationship-editor/release.js Outdated Show resolved Hide resolved
root/static/scripts/release-editor/edits.js Outdated Show resolved Hide resolved
root/static/scripts/tests/relationship-editor.js Outdated Show resolved Hide resolved
mwiencek
mwiencek requested changes Dec 9, 2019
View reviewed changes
Copy link
Member

@mwiencek mwiencek left a comment

Choose a reason for hiding this comment

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

This overlapped a lot with 1eb29e3 (which is now merged), so it'd make the diff easier to read if we rebased this PR on the latest master branch. (There will likely be some conflicts.)

root/static/scripts/edit/MB/edit.js Outdated Show resolved Hide resolved
root/static/scripts/edit/MB/edit.js Outdated Show resolved Hide resolved
root/static/scripts/edit/MB/edit.js Outdated Show resolved Hide resolved
root/static/scripts/edit/utility/guessFeat.js Outdated Show resolved Hide resolved
root/static/scripts/guess-case/MB/Control/GuessCase.js Outdated Show resolved Hide resolved
root/static/scripts/guess-case/MB/Control/GuessCase.js Outdated Show resolved Hide resolved
root/static/scripts/guess-case/MB/GuessCase/Handler/Base.js Outdated Show resolved Hide resolved
root/static/scripts/timeline.js Outdated Show resolved Hide resolved
@mwiencek
Copy link
Member

mwiencek commented Dec 9, 2019

I suggested some cases where keeping things on one line but using an arrow function would improve readability.

@reosarevok reosarevok added GCI PRs sent by GCI students Refactoring Refactoring-only PRs (eslint fixes etc) labels Dec 10, 2019
@mwiencek
Copy link
Member

This has been merged in 87f0ab5. Thank you! :)

@mwiencek mwiencek closed this Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwiencek mwiencek mwiencek approved these changes

@reosarevok reosarevok reosarevok approved these changes

Assignees
No one assigned
Labels
GCI PRs sent by GCI students Refactoring Refactoring-only PRs (eslint fixes etc)
Projects
None yet
Milestone
No milestone
3 participants
@voloshinskii @mwiencek @reosarevok