[ZEPPELIN-235] jscs in zeppelin-web by prabhjyotsingh · Pull Request #1139 · apache/zeppelin

prabhjyotsingh · 2016-07-06T10:36:27Z

What is this PR for?

Reopening the PR #229 for jscs in zeppelin-web, using following in .jscsrc

{
  "preset": "google",
  "maximumLineLength": {
  "value": 120,
  "allExcept": [ "comments", "regex"]
  }
}

What type of PR is it?

[Improvement]

What is the Jira issue?

ZEPPELIN-235

How should this be tested?

Try changing the indentation in any javascript file, and see an error being shown in the terminal on compile/build

Questions:

Does the licenses files need update? n/a
Is there breaking changes for older versions? n/a
Does this needs documentation? n/a

ghost · 2016-07-06T10:42:20Z

LGTM

r-kamath · 2016-07-06T10:47:11Z

LGTM

prabhjyotsingh · 2016-07-06T11:04:28Z

CI fails for Failed to run task: 'npm install --color=false' failed., re-triggered.

corneadoug · 2016-07-06T12:29:53Z

zeppelin-web/src/app/notebook/notebook.controller.js

  };

-  $scope.killSaveTimer = function() {
+    $scope.killSaveTimer = function() {


This indent and a few underneath looks a bit weird

corneadoug · 2016-07-06T12:36:14Z

The indent in notebook.controller.js is messed up

prabhjyotsingh · 2016-07-06T13:49:28Z

@corneadoug, have made the relevant changes.

It was coming because function($scope..... was is next line, and because of which build was failing with validateIndentation: Expected indentation of 4 characters.

corneadoug · 2016-07-07T06:19:32Z

@prabhjyotsingh Thanks for fixing it.
Looks awesome, let me test it a bit

corneadoug · 2016-07-07T06:23:05Z

zeppelin-web/Gruntfile.js

  ]);

  grunt.registerTask('build', [
+    'newer:jscs',


this should be jscs only in case of build.
newer:jscs should be used for the watch (around line 118 for basic .js and 125 for test files).

prabhjyotsingh · 2016-07-07T07:32:48Z

@corneadoug, have addressed your feedback.

# Conflicts: # zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

corneadoug · 2016-07-08T02:59:40Z

@prabhjyotsingh Tested, LGTM
CI failure is unrelated

prabhjyotsingh · 2016-07-08T09:09:36Z

Merging this if no more discussion.

### What is this PR for? #1151 has been created created before #1139 merge. #1151 was passing CI but it's failing in master with #1139 merged. ``` [INFO] --- frontend-maven-plugin:0.0.25:grunt (grunt build) zeppelin-web --- [INFO] Running 'grunt build --no-color' in /Users/moon/Projects/zeppelin/zeppelin-web [INFO] Running "jscs:all" (jscs) task [INFO] requireSpaceBeforeBinaryOperators: Operator = should not stick to preceding expression at src/components/saveAs/saveAs.service.js : [INFO] 21 | angular.element('body').append('<iframe id="SaveAsId" style="display: none"></iframe>'); [INFO] 22 | var frameSaveAs = angular.element('body > iframe#SaveAsId')[0].contentWindow; [INFO] 23 | content= BOM + content; [INFO] ---------------------^ [INFO] 24 | frameSaveAs.document.open('text/json', 'replace'); [INFO] 25 | frameSaveAs.document.write(content); [INFO] >> 1 code style errors found! [INFO] Warning: Task "jscs:all" failed. Use --force to continue. [INFO] [INFO] Aborted due to warnings. ``` ### What type of PR is it? Hot Fix ### Todos * [x] - Fix problem ### What is the Jira issue? ZEPPELIN-1138, ZEPPELIN-235 ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Lee moon soo <[email protected]> Closes #1165 from Leemoonsoo/fix_js and squashes the following commits: 3819ffb [Lee moon soo] add space (cherry picked from commit 0590ef3) Signed-off-by: Lee moon soo <[email protected]>

### What is this PR for? #1151 has been created created before #1139 merge. #1151 was passing CI but it's failing in master with #1139 merged. ``` [INFO] --- frontend-maven-plugin:0.0.25:grunt (grunt build) zeppelin-web --- [INFO] Running 'grunt build --no-color' in /Users/moon/Projects/zeppelin/zeppelin-web [INFO] Running "jscs:all" (jscs) task [INFO] requireSpaceBeforeBinaryOperators: Operator = should not stick to preceding expression at src/components/saveAs/saveAs.service.js : [INFO] 21 | angular.element('body').append('<iframe id="SaveAsId" style="display: none"></iframe>'); [INFO] 22 | var frameSaveAs = angular.element('body > iframe#SaveAsId')[0].contentWindow; [INFO] 23 | content= BOM + content; [INFO] ---------------------^ [INFO] 24 | frameSaveAs.document.open('text/json', 'replace'); [INFO] 25 | frameSaveAs.document.write(content); [INFO] >> 1 code style errors found! [INFO] Warning: Task "jscs:all" failed. Use --force to continue. [INFO] [INFO] Aborted due to warnings. ``` ### What type of PR is it? Hot Fix ### Todos * [x] - Fix problem ### What is the Jira issue? ZEPPELIN-1138, ZEPPELIN-235 ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Lee moon soo <[email protected]> Closes #1165 from Leemoonsoo/fix_js and squashes the following commits: 3819ffb [Lee moon soo] add space

@corneadoug

### What is this PR for? Reopening the PR apache#229 for jscs in zeppelin-web, using following in .jscsrc ``` { "preset": "google", "maximumLineLength": { "value": 120, "allExcept": [ "comments", "regex"] } } ``` ### What type of PR is it? [Improvement] ### What is the Jira issue? * [ZEPPELIN-235](https://issues.apache.org/jira/browse/ZEPPELIN-235) ### How should this be tested? Try changing the indentation in any javascript file, and see an error being shown in the terminal on compile/build ### Questions: * Does the licenses files need update? n/a * Is there breaking changes for older versions? n/a * Does this needs documentation? n/a Author: Prabhjyot Singh <[email protected]> Closes apache#1139 from prabhjyotsingh/ZEPPELIN-235 and squashes the following commits: 9cc3cde [Prabhjyot Singh] resove merge errors a274efa [Prabhjyot Singh] Merge remote-tracking branch 'origin/master' into ZEPPELIN-235 c16d2c7 [Prabhjyot Singh] addressing @corneadoug feedback bffd134 [Prabhjyot Singh] revert 4 space changes for notebook.controller.js 02de55d [Prabhjyot Singh] jscs in zeppelin-web

### What is this PR for? apache#1151 has been created created before apache#1139 merge. apache#1151 was passing CI but it's failing in master with apache#1139 merged. ``` [INFO] --- frontend-maven-plugin:0.0.25:grunt (grunt build) zeppelin-web --- [INFO] Running 'grunt build --no-color' in /Users/moon/Projects/zeppelin/zeppelin-web [INFO] Running "jscs:all" (jscs) task [INFO] requireSpaceBeforeBinaryOperators: Operator = should not stick to preceding expression at src/components/saveAs/saveAs.service.js : [INFO] 21 | angular.element('body').append('<iframe id="SaveAsId" style="display: none"></iframe>'); [INFO] 22 | var frameSaveAs = angular.element('body > iframe#SaveAsId')[0].contentWindow; [INFO] 23 | content= BOM + content; [INFO] ---------------------^ [INFO] 24 | frameSaveAs.document.open('text/json', 'replace'); [INFO] 25 | frameSaveAs.document.write(content); [INFO] >> 1 code style errors found! [INFO] Warning: Task "jscs:all" failed. Use --force to continue. [INFO] [INFO] Aborted due to warnings. ``` ### What type of PR is it? Hot Fix ### Todos * [x] - Fix problem ### What is the Jira issue? ZEPPELIN-1138, ZEPPELIN-235 ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: Lee moon soo <[email protected]> Closes apache#1165 from Leemoonsoo/fix_js and squashes the following commits: 3819ffb [Lee moon soo] add space

jscs in zeppelin-web

02de55d

prabhjyotsingh force-pushed the ZEPPELIN-235 branch from a278e4b to 02de55d Compare July 6, 2016 11:02

corneadoug reviewed Jul 6, 2016
View reviewed changes

revert 4 space changes for notebook.controller.js

bffd134

corneadoug reviewed Jul 7, 2016
View reviewed changes

addressing @corneadoug feedback

c16d2c7

prabhjyotsingh added 2 commits July 7, 2016 22:22

Merge remote-tracking branch 'origin/master' into ZEPPELIN-235

a274efa

# Conflicts: # zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

resove merge errors

9cc3cde

prabhjyotsingh force-pushed the ZEPPELIN-235 branch from 081f749 to 9cc3cde Compare July 7, 2016 18:53

asfgit closed this in 5556f81 Jul 8, 2016

Leemoonsoo mentioned this pull request Jul 11, 2016

[HOTFIX] build fail after merge #1151 #1165

Closed

1 task

prabhjyotsingh deleted the ZEPPELIN-235 branch February 25, 2018 03:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ZEPPELIN-235] jscs in zeppelin-web#1139

[ZEPPELIN-235] jscs in zeppelin-web#1139
prabhjyotsingh wants to merge 5 commits intoapache:masterfrom
prabhjyotsingh:ZEPPELIN-235

prabhjyotsingh commented Jul 6, 2016

Uh oh!

ghost commented Jul 6, 2016

Uh oh!

r-kamath commented Jul 6, 2016

Uh oh!

prabhjyotsingh commented Jul 6, 2016

Uh oh!

corneadoug Jul 6, 2016

Uh oh!

corneadoug commented Jul 6, 2016

Uh oh!

prabhjyotsingh commented Jul 6, 2016

Uh oh!

corneadoug commented Jul 7, 2016

Uh oh!

corneadoug Jul 7, 2016

Uh oh!

prabhjyotsingh commented Jul 7, 2016

Uh oh!

corneadoug commented Jul 8, 2016

Uh oh!

prabhjyotsingh commented Jul 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

prabhjyotsingh commented Jul 6, 2016

What is this PR for?

What type of PR is it?

What is the Jira issue?

How should this be tested?

Questions:

Uh oh!

ghost commented Jul 6, 2016

Uh oh!

r-kamath commented Jul 6, 2016

Uh oh!

prabhjyotsingh commented Jul 6, 2016

Uh oh!

corneadoug Jul 6, 2016

Choose a reason for hiding this comment

Uh oh!

corneadoug commented Jul 6, 2016

Uh oh!

prabhjyotsingh commented Jul 6, 2016

Uh oh!

corneadoug commented Jul 7, 2016

Uh oh!

corneadoug Jul 7, 2016

Choose a reason for hiding this comment

Uh oh!

prabhjyotsingh commented Jul 7, 2016

Uh oh!

corneadoug commented Jul 8, 2016

Uh oh!

prabhjyotsingh commented Jul 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants