Feature/eslint #831

jason-fox · 2019-11-30T12:54:44Z

Replace jshint with eslint
Add prettier code formatting
Add husky and lint-staged

Incorporates and supercedes #753

Related: #830

…eslint

jason-fox · 2020-10-06T07:29:55Z

@fgalan @AlvaroVega -

Is there anything now blocking this PR from being reviewed and merged?

fgalan · 2020-10-06T07:46:04Z

@fgalan @AlvaroVega -

Is there anything now blocking this PR from being reviewed and merged?

As far as I understand by #830 (comment), this PR is waiting for others to be merged first (#849 and maybe #854)

jason-fox · 2020-10-06T09:25:36Z

@fgalan - Given the lack of movement on #849, I guess it would make more sense to move this up the queue. #854 is dependent on #849 so would come after the others.

So I'd suggest processing in the following order - #831 , #849 then #854

It's just that there isn't much of a queue at the moment (since the last period of activity has cleared a of of small PRs) so it would make sense to use this opportunity to get the broad ES6 change in whilst the "lull" occurs.

fgalan · 2020-10-07T15:01:07Z

@fgalan - Given the lack of movement on #849, I guess it would make more sense to move this up the queue. #854 is dependent on #849 so would come after the others.

So I'd suggest processing in the following order - #831 , #849 then #854

It's just that there isn't much of a queue at the moment (since the last period of activity has cleared a of of small PRs) so it would make sense to use this opportunity to get the broad ES6 change in whilst the "lull" occurs.

I have updated the table at #830 (comment) based on your feedback. In addition, I have asked other PRs authors about what they think about merging #831 before theirs.

If they agree (or if they don't answer in a reasonable time, assuming in that case they are missing in action :) I think we can finally progress and eventually merge PR #831

jason-fox · 2020-10-27T16:35:02Z

If they agree (or if they don't answer in a reasonable time, assuming in that case they are missing in action :) I think we can finally progress and eventually merge PR #831

@fgalan @AlvaroVega - Any chance this will be actioned before its upcoming first birthday? 🍰

AlvaroVega · 2020-10-27T16:42:44Z

lib/services/common/alarmManagement.js

@@ -38,7 +37,7 @@ function raise(alarmName, description) {
    if (!alarmRepository[alarmName]) {
        alarmRepository[alarmName] = {
            name: alarmName,
-            description: description
+            description


are your sure about this change?

EsLint and Prettier are. This is standard syntax for ES6.

AlvaroVega · 2020-10-27T16:43:09Z

lib/services/commands/commandService.js

-        config.getCommandRegistry().removeFromDate(
-            Date.now() - config.getConfig().pollingExpiration,
-            function(error, results) {
+        config


I'm not sure if this way is more easy to read....

Automated change - take it up with the guys at Prettier. 😄 This syntax has already been applied across the IoT Agents themselves the only hold out is the IoT-Node-lib so this is the one that isn't standardized.

jason-fox · 2020-11-10T11:28:56Z

@fgalan @AlvaroVega -

With the OPC-UA template change #920 , it looks like there are no objections left in the table here.
Is there anything now blocking this PR from being reviewed and merged so that #830 can finally be closed.

fgalan · 2020-11-10T12:49:31Z

@fgalan @AlvaroVega -

With the OPC-UA template change #920 , it looks like there are no objections left in the table here.

I have reviewed and the only pending item is #601, but given the author seems to be MIA (Missing In Action :) as he doesn't answer to the question about adapting his PR after merging the lint changes, I think we can skip it and GO.

Is there anything now blocking this PR from being reviewed and merged so that #830 can finally be closed.

We are near to close a new version of the iotagent-node-lib (along with some other agents). We will wait until that release (we hope that some days or a week maybe) before merging the eslint contribution (in fact, the eslint contribution could be the first PR to be merged in the new version development cycle).

fgalan · 2020-11-16T12:47:17Z

We are near to close a new version of the iotagent-node-lib (along with some other agents). We will wait until that release (we hope that some days or a week maybe) before merging the eslint contribution (in fact, the eslint contribution could be the first PR to be merged in the new version development cycle).

2.14.0 has been already released (https://github.com/telefonicaid/iotagent-node-lib/releases/tag/2.14.0)

Thus, this PR would be ready to be merged, once the conflict with master gets solved.

# Conflicts: # CHANGES_NEXT_RELEASE # lib/services/northBound/deviceProvisioningServer.js

fgalan · 2020-11-17T14:30:56Z

lib/errors.js

- * or (at your option) any later version.
+ * or {
+ constructor(at your option) any later version.


Fixed ae4416e

fgalan · 2020-11-17T14:47:40Z

.eslintrc.json

+  "plugins": ["prettier"],
+  "rules": {
+    "prettier/prettier": "error",
+    "no-shadow": 0,


Comparing with the .eslintrc.json file in IOTA-JSON ~~.eslintrc.json~~ repo, this setting has been added here.

Which effect does it have in the eslint rules?

Same reasoning it was added here - the latest version of eslint complains about Telefonica style callbacks in the code (it wants the callback variable name to be unique) hence the shadowed variable name rule must be dropped. This is only due to the updates to a more modern Eslint

Thanks for the clarification.

So, I understand that "no-shadow": 0 is the desired setting and the repos not using it (if any) should be fixed. Out of the scope of this PR, of course.

NTC

CHANGES_NEXT_RELEASE

fgalan · 2020-11-17T14:54:51Z

package.json

+    "prettier": "prettier --config .prettierrc.json --write '**/**/**/**/*.js' '**/**/**/*.js' '**/**/*.js' '**/*.js' '*.js'",
+    "prettier:text": "prettier 'README.md' 'doc/*.md' 'doc/**/*.md' --no-config --tab-width 4 --print-width 120 --write --prose-wrap always",


Are the new targets (prettier and prettier:text) included in the corresponding .md describing the different targets? Maybe yes, but it's a bit hard to find it in so large PR :) Could you help providing a pointer, please?

Fixed ea6d399

Co-authored-by: Fermín Galán Márquez <[email protected]>

fgalan

LGTM

It has been hard, but in the end, the PR is fine and we have found a good lull moment for it. Thanks for all your contributions!

jason-fox · 2020-11-18T15:47:35Z

Merge commit 'master' into feature/842_ngsi_ld - Add ESlint #937 applies ESLint to Adding NGSI-LD support #842.
EsLint has also been applied to For NGSI-v2, ensure GeoJSON is correctly encode in the request. #854 and Feature/mixed #930 which are dependent on Adding NGSI-LD support #842 which should be reviewed/processed next.
EsLint has also been applied to Cache Mongo-DB calls (in-memory and/or Redis) #926, which fixes one outstanding review comment

jason-fox added 2 commits November 30, 2019 12:31

Integrate EsLint + Prettier

7028300

Add CNR

206886c

jason-fox mentioned this pull request Nov 30, 2019

Should every IoT Agent align to use eslint #830

Closed

jason-fox and others added 13 commits November 30, 2019 14:19

Fixing test expectations

a5334ea

Fixing test expectations

e4df205

Merge commit 'd9c5ac9af583e7610eaafa5baa0f4abb1aead5c2' into feature/…

433dd6c

…eslint

Merge commit '25b690aa97059ba3e2306f300ca3647486134c18' into feature/…

7bf4d03

…eslint

Add editorconfig

cd9475a

Update coding guidelines to include eslint and editorconfig

cbe8e4d

Amending text

876ca4b

Capitalize ESLint

0bd74e7

Update .editorconfig

5e839d9

Merge branch 'master' into feature/eslint

325aaab

Merge branch 'master' into feature/eslint

288a22e

Remove package lock.

4820c76

Merge branch 'master' into feature/eslint

61bcd0a

This was referenced Feb 11, 2020

Prepend NGSI-v1 to the v1 tests, NGSI-v2 to the v2 tests #840

Merged

Basic NGSI-LD active measures support #841

Merged

jason-fox added 2 commits February 26, 2020 23:12

Merge branch 'master' into feature/eslint

9f261d9

Re-add CNR

fe6f3dc

jason-fox mentioned this pull request Mar 4, 2020

Update NGSI support to use ES6 syntax. #850

Merged

jason-fox added 4 commits June 22, 2020 18:01

Merge commit '2f6509ba67eca09486161092fcef9ab0430f75a3' into feature/…

113b4ac

…eslint

Remove package lock

14e64a0

Merge commit '4519d15018205914941decac4bc44ae2f2bc6c73' into feature/…

ee7b6a0

…eslint

Merge branch 'master' into feature/eslint

20868dd

jason-fox mentioned this pull request Jul 15, 2020

Add prettier code formatting #753

Closed

Fix broken test from merge

c35b0ba

jason-fox mentioned this pull request Jul 29, 2020

When should dependencies be updated? telefonicaid/iotagent-ul#434

Merged

jason-fox added 2 commits September 10, 2020 19:20

Merge branch 'master' into feature/eslint

818d8a1

Run prettier, fix docs.

c0a6b96

This was referenced Oct 7, 2020

Including Lazy, Commands, Internal and Static Attributes in payload to IoT Agent Manager #793

Merged

[WIP] northBound server in HTTPS and Verify ContextBroker Certificate #601

Open

jason-fox added 3 commits October 7, 2020 17:07

Merge branch 'master' into feature/eslint

aabb4ea

Increase timeout

8de0be2

Merge 'master' into feature/eslint

32e84af

AlvaroVega reviewed Oct 27, 2020

View reviewed changes

jason-fox added 2 commits October 27, 2020 18:17

Merge branch 'master' into feature/eslint

dfce339

Merge commit 'master' into feature/eslint

6b4c511

Merge branch 'master' into feature/eslint

924c3a9

# Conflicts: # CHANGES_NEXT_RELEASE # lib/services/northBound/deviceProvisioningServer.js

fgalan reviewed Nov 17, 2020

View reviewed changes

CHANGES_NEXT_RELEASE Outdated Show resolved Hide resolved

fgalan reviewed Nov 17, 2020

View reviewed changes

jason-fox and others added 3 commits November 18, 2020 09:41

Update CHANGES_NEXT_RELEASE

68cebf8

Co-authored-by: Fermín Galán Márquez <[email protected]>

Fix typo

ae4416e

Update documentation.

ea6d399

fgalan approved these changes Nov 18, 2020

View reviewed changes

fgalan merged commit 510ae37 into telefonicaid:master Nov 18, 2020

jason-fox mentioned this pull request Nov 18, 2020

Merge commit 'master' into feature/842_ngsi_ld - Add ESlint #937

Merged

jason-fox deleted the feature/eslint branch November 20, 2020 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/eslint #831

Feature/eslint #831

jason-fox commented Nov 30, 2019

jason-fox commented Oct 6, 2020

fgalan commented Oct 6, 2020

jason-fox commented Oct 6, 2020

fgalan commented Oct 7, 2020

jason-fox commented Oct 27, 2020

AlvaroVega Oct 27, 2020

jason-fox Oct 27, 2020

AlvaroVega Oct 27, 2020

jason-fox Oct 27, 2020

jason-fox commented Nov 10, 2020

fgalan commented Nov 10, 2020 •

edited

Loading

fgalan commented Nov 16, 2020

fgalan Nov 17, 2020

jason-fox Nov 18, 2020

fgalan Nov 17, 2020 •

edited

Loading

jason-fox Nov 18, 2020 •

edited

Loading

fgalan Nov 18, 2020

fgalan Nov 17, 2020

jason-fox Nov 18, 2020

fgalan left a comment

jason-fox commented Nov 18, 2020

		"prettier": "prettier --config .prettierrc.json --write '////.js' '///.js' '//.js' '/.js' '*.js'",
		"prettier:text": "prettier 'README.md' 'doc/.md' 'doc//.md' --no-config --tab-width 4 --print-width 120 --write --prose-wrap always",

Feature/eslint #831

Feature/eslint #831

Conversation

jason-fox commented Nov 30, 2019

jason-fox commented Oct 6, 2020

fgalan commented Oct 6, 2020

jason-fox commented Oct 6, 2020

fgalan commented Oct 7, 2020

jason-fox commented Oct 27, 2020

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

jason-fox commented Nov 10, 2020

fgalan commented Nov 10, 2020 • edited Loading

fgalan commented Nov 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

jason-fox Nov 18, 2020 • edited Loading

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

fgalan left a comment

Choose a reason for hiding this comment

jason-fox commented Nov 18, 2020

fgalan commented Nov 10, 2020 •

edited

Loading

fgalan Nov 17, 2020 •

edited

Loading

jason-fox Nov 18, 2020 •

edited

Loading