Disallow using eslint disable statement for camelcase #9482

nishantwrp · 2020-06-10T23:04:40Z

Overview

This PR fixes or fixes part of NA.
This PR does the following: Disallow using eslint disable statement for camelcase

Essential 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 linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

PR Pointers

Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays
Never force push. If you do, your PR will be closed.

oppiabot · 2020-06-10T23:04:43Z

Assigning @kevinlee12 for the first-pass review of this pull request. Thanks!

nishantwrp · 2020-06-11T07:21:00Z

scripts/linters/js_ts_linter.py

+            'typings/guppy-defs-b5055b963fdbea5c6c1e92dbf58fdaf3ea0cd8ba.d.ts',
+            'core/templates/services/UpgradedServices.ts']


These were needed because these files had variables in snake_case that couldn't use quotes.

codecov · 2020-06-11T08:36:55Z

Codecov Report

Merging #9482 into develop will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #9482   +/-   ##
========================================
  Coverage    55.75%   55.75%           
========================================
  Files          966      966           
  Lines        38605    38605           
  Branches      4719     4719           
========================================
  Hits         21523    21523           
  Misses       15775    15775           
  Partials      1307     1307

Flag	Coverage Δ
#frontend	`55.75% <100.00%> (ø)`

Impacted Files	Coverage Δ
...nts/ck-editor-helpers/ck-editor-4-rte.directive.ts	`4.71% <ø> (ø)`
...tom-forms-directives/apply-validation.directive.ts	`100.00% <ø> (ø)`
...plates/domain/exploration/SolutionObjectFactory.ts	`94.74% <ø> (ø)`
...omain/statistics/LearnerAnswerInfoObjectFactory.ts	`100.00% <ø> (ø)`
...topics-and-skills-dashboard-backend-api.service.ts	`69.23% <ø> (ø)`
.../contribution-opportunities-backend-api.service.ts	`85.00% <ø> (ø)`
...b/services/state-improvement-suggestion.service.ts	`89.66% <ø> (ø)`
...ge/services/exploration-recommendations.service.ts	`37.84% <ø> (ø)`
...on-player-page/services/stats-reporting.service.ts	`30.26% <ø> (ø)`
core/templates/services/playthrough.service.ts	`77.86% <ø> (ø)`
... and 3 more

vojtechjelinek

LGTM! Just a two nits.

vojtechjelinek · 2020-06-11T11:51:28Z

core/templates/domain/exploration/SolutionObjectFactory.ts

@@ -149,7 +144,6 @@ export class SolutionObjectFactory {
  // TODO(ankita240796): Remove the bracket notation once Angular2 gets in.
  /* eslint-disable dot-notation */
  createNew(
-  /* eslint-enable dot-notation */


Why remove this?

there was no use of this.

Okay, I was just confused since previously the stuf on line 150 was kept.

vojtechjelinek · 2020-06-11T11:51:39Z

core/templates/domain/exploration/SolutionObjectFactory.ts

@@ -136,7 +132,6 @@ export class SolutionObjectFactory {
    private shof: SubtitledHtmlObjectFactory,
    private ehfs: ExplorationHtmlFormatterService) {}
  createFromBackendDict(solutionBackendDict: SolutionBackendDict): Solution {
-  /* eslint-enable dot-notation */


Why remove this?

brianrodri · 2020-06-11T13:45:29Z

LGTM for codeowner files

DubeySandeep · 2020-06-11T15:36:13Z

scripts/linters/js_ts_linter.py

@@ -56,6 +56,20 @@
 _MESSAGE_TYPE_SUCCESS = 'SUCCESS'
 _MESSAGE_TYPE_FAILED = 'FAILED'

+_BANNED_ESLINT_DISABLE_STATEMENTS = [
+    {
+        'pattern': r'eslint-(disable|enable)(-next-line)? camelcase',


Can we use BAD_PATTERNS_JS_AND_TS_REGEXP instead of creating a new block here?

done! didn't know we had something like this.

DubeySandeep

LGTM for the codeowner files*

nishantwrp · 2020-06-12T19:41:47Z

@sagangwee @nithusha21 @aks681 @bansalnitish @Hudda ptal!

sagangwee

@nishantwrp LGTM for codeowners. Thanks!

Hudda

LGTM as codeowner. Thanks @nishantwrp

bansalnitish

LGTM!

nithusha21

LGTM as a codeowner!

aks681

Lgtm as codeowner

…heck

nishantwrp · 2020-06-15T18:40:42Z

@vojtechjelinek I think this can be merged now!

Disallow using eslint disable statement for camelcase

b071bd6

nishantwrp added the PR CHANGELOG: Developer experience -- @kevinlee12 label Jun 10, 2020

nishantwrp requested review from aks681, ankita240796, bansalnitish, brianrodri, DubeySandeep, Hudda, kevintab95, nithusha21, sagangwee and vojtechjelinek as code owners June 10, 2020 23:04

nishantwrp assigned vojtechjelinek Jun 10, 2020

oppiabot bot assigned kevinlee12 Jun 10, 2020

nishantwrp unassigned kevinlee12 Jun 10, 2020

nishantwrp commented Jun 11, 2020

View reviewed changes

vojtechjelinek approved these changes Jun 11, 2020

View reviewed changes

vojtechjelinek assigned vojtechjelinek, brianrodri, sagangwee, kevintab95, nithusha21, ankita240796, DubeySandeep, aks681, bansalnitish and Hudda and unassigned vojtechjelinek Jun 11, 2020

brianrodri removed their assignment Jun 11, 2020

nishantwrp removed their assignment Jun 11, 2020

DubeySandeep reviewed Jun 11, 2020

View reviewed changes

DubeySandeep assigned nishantwrp and unassigned DubeySandeep Jun 11, 2020

move to bad patterns

ac8eb27

nishantwrp assigned DubeySandeep and unassigned nishantwrp Jun 11, 2020

nishantwrp requested a review from DubeySandeep June 11, 2020 22:19

DubeySandeep approved these changes Jun 12, 2020

View reviewed changes

DubeySandeep removed their assignment Jun 12, 2020

sagangwee approved these changes Jun 13, 2020

View reviewed changes

sagangwee removed their assignment Jun 13, 2020

Hudda approved these changes Jun 13, 2020

View reviewed changes

Hudda removed their assignment Jun 13, 2020

bansalnitish approved these changes Jun 13, 2020

View reviewed changes

bansalnitish removed their assignment Jun 13, 2020

nithusha21 approved these changes Jun 13, 2020

View reviewed changes

nithusha21 removed their assignment Jun 13, 2020

aks681 approved these changes Jun 14, 2020

View reviewed changes

aks681 assigned nishantwrp and unassigned aks681 Jun 14, 2020

Merge remote-tracking branch 'upstream/develop' into eslint-disable-c…

d76d53e

…heck

nishantwrp assigned vojtechjelinek and unassigned nishantwrp Jun 15, 2020

vojtechjelinek merged commit 32e12b9 into oppia:develop Jun 15, 2020

shavavo pushed a commit to shavavo/oppia that referenced this pull request Aug 20, 2020

Disallow using eslint disable statement for camelcase (oppia#9482)

ca7e575

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow using eslint disable statement for camelcase #9482

Disallow using eslint disable statement for camelcase #9482

nishantwrp commented Jun 10, 2020

oppiabot bot commented Jun 10, 2020

nishantwrp Jun 11, 2020

codecov bot commented Jun 11, 2020 •

edited

vojtechjelinek left a comment

vojtechjelinek Jun 11, 2020

nishantwrp Jun 11, 2020

vojtechjelinek Jun 11, 2020

vojtechjelinek Jun 11, 2020

nishantwrp Jun 11, 2020

brianrodri commented Jun 11, 2020

DubeySandeep Jun 11, 2020

nishantwrp Jun 11, 2020

DubeySandeep left a comment

nishantwrp commented Jun 12, 2020

sagangwee left a comment

Hudda left a comment

bansalnitish left a comment

nithusha21 left a comment

aks681 left a comment

nishantwrp commented Jun 15, 2020

		'typings/guppy-defs-b5055b963fdbea5c6c1e92dbf58fdaf3ea0cd8ba.d.ts',
		'core/templates/services/UpgradedServices.ts']

Disallow using eslint disable statement for camelcase #9482

Disallow using eslint disable statement for camelcase #9482

Conversation

nishantwrp commented Jun 10, 2020

Overview

Essential Checklist

PR Pointers

oppiabot bot commented Jun 10, 2020

Choose a reason for hiding this comment

codecov bot commented Jun 11, 2020 • edited

Codecov Report

vojtechjelinek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brianrodri commented Jun 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DubeySandeep left a comment

Choose a reason for hiding this comment

nishantwrp commented Jun 12, 2020

sagangwee left a comment

Choose a reason for hiding this comment

Hudda left a comment

Choose a reason for hiding this comment

bansalnitish left a comment

Choose a reason for hiding this comment

nithusha21 left a comment

Choose a reason for hiding this comment

aks681 left a comment

Choose a reason for hiding this comment

nishantwrp commented Jun 15, 2020

codecov bot commented Jun 11, 2020 •

edited