NotificationsYou must be signed in to change notification settings
Fork20.6k
Star59.5k

Build: Correct code indentations based on jQuery Style Guide#4672

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mgol merged 1 commit intojquery:masterfromwonseop:idiom

May 5, 2020

Merged

Build: Correct code indentations based on jQuery Style Guide#4672

mgol merged 1 commit intojquery:masterfromwonseop:idiom

May 5, 2020

Conversation

Copy link

Contributor

wonseop commentedApr 17, 2020•
edited
Loading

Summary

Correct code indentations based on Core Style Guide
(https://contribute.jquery.org/style-guide/js/#spacing)
Add rules tosrc/.eslintrc.json to enable "enforcing consistent indentation",
with minimal changes to the current code.

Checklist

All authors have signed the CLA athttps://cla.js.foundation/jquery/jquery
~~New tests have been added to show the fix or feature works~~
Grunt build and unit tests pass locally with these changes
~~If needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com~~

wonseop force-pushed theidiom branch from2444ad2 toe0f96d0Compare

April 17, 2020 02:54

Copy link

Member

mgol commentedApr 17, 2020

Thanks for the PR!

I'm wondering if we shouldn't just enable theindent option, at least insrc/.eslintrc.json. I'm sure there are some violations but I've heard that rule has improved a lot recently so maybe it'd be feasible for us to use it. That way we won't need to apply such small indentation fixes but we'll be sure it's fine all the time.

Docs for the indent rule:https://eslint.org/docs/rules/indent.

Would you like to have a look at that?

wonseop force-pushed theidiom branch 2 times, most recently from7b84c3e todfed21eCompare

April 23, 2020 11:11

mgol requested changes

Apr 23, 2020

View reviewed changes

Copy link

Member

mgol left a comment

Choose a reason for hiding this comment

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

I added a few comments. Overall, this looks great! I was afraid we'd need more source changes.

src/.eslintrc.json OutdatedShow resolvedHide resolved

src/.eslintrc.jsonShow resolvedHide resolved

src/deprecated/event.js OutdatedShow resolvedHide resolved

src/serialize.js OutdatedShow resolvedHide resolved

wonseop changed the title~~Correct code indentation according to Core Style Guide~~Correct code indentation based on Core Style Guide

Apr 24, 2020

wonseop force-pushed theidiom branch 2 times, most recently from0e2964d to8bcc817Compare

April 24, 2020 07:33

mgol reviewed

Apr 24, 2020

View reviewed changes

src/deprecated/event.jsShow resolvedHide resolved

mgol requested changes

Apr 24, 2020

View reviewed changes

Copy link

Member

mgol left a comment

Choose a reason for hiding this comment

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

Please rebase your branch over latest master. There have been some changes to the ESLint config recently so I'd like to be sure it passes the lint.

wonseop force-pushed theidiom branch from8bcc817 tof00ca7fCompare

April 24, 2020 09:53

Correct code indentations

76fb3ef

1. Correct code indentations based on Core Style Guide.   (https://contribute.jquery.org/style-guide/js/#spacing)2. Add rules to "src/.eslintrc.json" to enable "enforcing consistent indentation",   with minimal changes to the current code.

wonseop force-pushed theidiom branch fromf00ca7f to76fb3efCompare

April 27, 2020 01:20

wonseop changed the title~~Correct code indentation based on Core Style Guide~~Build: Correct code indentations based on Core Style Guide.

Apr 27, 2020

wonseop requested a review frommgol

April 27, 2020 01:53

mgol approved these changes

Apr 27, 2020

View reviewed changes

Copy link

Member

mgol left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me!

mgol added Build Discuss in Meeting

Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting.

Needs review labels

Apr 27, 2020

timmywil removed the Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label

Apr 28, 2020

Copy link

Member

mgol commentedApr 29, 2020

@timmywil We didn't discuss it yet, I wanted a final team approval so let me add the label back.

mgol added the Discuss in MeetingReserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label

Apr 29, 2020

timmywil approved these changes

May 4, 2020

View reviewed changes

Copy link

Member

timmywil left a comment

Choose a reason for hiding this comment

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

LGTM

mgol removed Discuss in Meeting

Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting.

Needs review labels

May 4, 2020

mgol added this to the4.0.0 milestone

May 4, 2020

mgol changed the title~~Build: Correct code indentations based on Core Style Guide.~~Build: Correct code indentations based on jQuery Style Guide

May 5, 2020

mgol merged commit3d62d57 intojquery:master

May 5, 2020

mgol pushed a commit to mgol/jquery that referenced this pull request

May 5, 2020

Build: Correct code indentations based on jQuery Style Guide

9b8cadc

1. Correct code indentations based on jQuery Style Guide   (contribute.jquery.org/style-guide/js/#spacing).2. Add rules to "src/.eslintrc.json" to enable "enforcing consistent   indentation", with minimal changes to the current code.Closesjquerygh-4672(cherry picked from3d62d57)

mgol removed this from the4.0.0 milestone

May 5, 2020

mgol added this to the3.6.0 milestone

May 5, 2020

Copy link

Member