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

Introduce eslint: space-before-blocks rule #6048

Merged
merged 1 commit into from
Jan 4, 2019
Merged

Introduce eslint: space-before-blocks rule #6048

merged 1 commit into from
Jan 4, 2019

Conversation

apb7
Copy link
Contributor

@apb7 apb7 commented Jan 3, 2019

Explanation

Changes have been made as suggested here:
#5842 (comment)

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

@apb7
Introduce eslint: space-before-blocks rule
985e32e 
Changes have been made as suggested here:
oppia#5842 (comment)
@apb7 apb7 requested a review from nithusha21 January 3, 2019 16:22
@apb7 apb7 requested a review from seanlip as a code owner January 3, 2019 16:22
@apb7 apb7 mentioned this pull request Jan 3, 2019
Merged
nithusha21
nithusha21 approved these changes Jan 3, 2019
View reviewed changes
Copy link
Contributor

@nithusha21 nithusha21 left a comment

Choose a reason for hiding this comment

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

Hi @apb7 LGTM. Just one clarification. Thanks for fixing this!

Feel free to merge after answering the question, and travis passes :)

core/templates/dev/head/domain/exploration/ContentIdsToAudioTranslationsObjectFactorySpec.js
});
it(
'should toggle needs update attribute in a given content id', function() {
citat.toggleNeedsUpdateAttribute('content', 'hi');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the extra indent expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, eslint produces an error for indentation if line 254 (and the rest of the block) is at the same level as line 253. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh OK. Thanks for confirming :)

@nithusha21 nithusha21 removed their assignment Jan 3, 2019
@nithusha21
Copy link
Contributor

Oh just realized a codeowner approval is needed. Will await Sean's review :)

seanlip
seanlip approved these changes Jan 4, 2019
View reviewed changes
Copy link
Member

@seanlip seanlip left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for delay, and thanks for the PR!

@seanlip seanlip merged commit 718d4d0 into oppia:develop Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nithusha21 nithusha21 nithusha21 approved these changes

@seanlip seanlip seanlip approved these changes

Assignees

@seanlip seanlip

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@apb7 @nithusha21 @seanlip