Add prettier code formatting #753

jason-fox · 2019-01-28T11:41:29Z

Adds prettier and husky. All JavaScript files are autoformated on commit using the Git Hooks hook.

running npm run prettier will format all files with standardized whitespace.

* Prettier will always double quote a string containing an apostrophe.

fgalan · 2019-01-28T17:00:26Z

package.json

@@ -57,12 +59,26 @@
  },
  "devDependencies": {
    "coveralls": "~3.0.2",
+    "husky": "^1.1.0",


Please use ~ dependencies (instead of ^)

Fixed 20bd137

I see husky has been fixed, but some others still:

"lint-staged": "^7.3.0", "prettier": "^1.14.2",

Could you fix them in the same way, pls?

Fixed 3536dbf

fgalan · 2019-01-28T17:02:33Z

package.json

    "should": "13.2.3",
    "timekeeper": "2.1.2",
    "watch": "~1.0.2"
+  },
+  "husky": {


How this exactly works? I mean:

Does some configuration needs to be do on the "Settings / Webhook" sections in the repository to have this working?

How the "autoformated on commit" works? Adding an additional commit automatically with the fix?

Kind of, Husky builds a local precommit hook for you on your local instance - there is no need to add anything extra to the repository on GitHub. It means that the code gets properly formatted prior commit.

see:

https://davidwalsh.name/prevent-bad-commits-husky

https://prettier.io/docs/en/precommit.html

Thanks for the links! I didn't know about pre-commit git hooks. Seems cool :)

NTC

fgalan · 2019-01-28T17:04:28Z

package.json

@@ -33,6 +34,7 @@
    "test": "mocha --recursive 'test/**/*.js' --reporter spec --timeout 3000 --ui bdd --exit",
    "test:watch": "npm run test -- -w ./lib",
    "lint": "jshint lib/ --config .jshintrc && jshint test/ --config test/.jshintrc",
+    "prettier": "prettier --single-quote --trailing-comma es5 --write **/*.js *.js",


The new npm target should have its corresponding entry under https://github.com/telefonicaid/iotagent-node-lib/tree/master/doc/usermanual.md#development-documentation

Fixed 75c0c54 - Of course you shouldn't need to run this manually as Husky will do it for you.

Looking to prettier CLI options, I'd suggest to included --tab-width 4 to keep the current indentation style.

I'd like to start a passionate discussion on indentation length ;), but the fact is that this repository code uses 4 from its very begining, so let's keep that.

Fixed a4979bd + 998f379 - This should make the diff less messy.

On the whole I've gone with the defaults suggested by prettier themselves, but double/single quote is repo preference and trailing-comma es5 is a step to the future (it adds the trailing quote only where ecma5 accepts it

Ideally once node 6 support is deprecated this can be updated to be consistent for ecma6 and use let and const and arrow functions and so on, but at least it is a start (and won't break the code)

fgalan · 2019-01-28T17:10:44Z

I have had a quick look to the diff of changes but it is very hard to get the detail in a +1,189 −1,005 lines PR :)

To check if I'm understand correctly... it seems the only change is to standarize the indent to 2 whitespaces in each level (changing from current 4 spaces indent). Is that all? Or is there any other change done by this prettier library?

jason-fox · 2019-01-29T08:51:48Z

it seems the only change is to standarize the indent to 2 whitespaces in each level (changing from current 4 spaces indent). Is that all? Or is there any other change done by this prettier library?

Basically yes, prettier is an opinionated code formatter, it will standardize whitespace, break up long lines etc, but deliberately doesn't give you too many options to play with.

Prettier needs to be run once over the whole codebase, the Husky lint-staged will do it automatically over each commit thereafter, so changes will be minimised and easier review since there won't be any extraneous whitespace or odd formatting within the commits.

fgalan · 2019-01-30T11:37:14Z

package.json

+  },
+  "lint-staged": {
+    "*.js": [
+      "prettier --parser flow --single-quote --trailing-comma es5 --write",


It would be great to avoid sentece repetition and just use:

"npm prettier",

However, I've realized that the sentence is not exaclly the same, --write vs. --write **/*.js *.js... Is there any solution to "factorize" common sentence in just one place to be used in both cases?

If there is I haven't found it yet. lint-staged is running prettier across each individual JavaScript file within the commit, the npm run prettier script is a convenience function for running across all files - it shouldn't need to be used much, but I find it useful to format my files locally whilst I'm working.

Ok. Pitty.

NTC

fgalan

The only pending thing seems to be changing ^ -> ~ in a couple of new dependencies in packages.json. Appart from that, LGTM.

In the meanwhile, passing the ball to @dcalvoalonso to get also his opinion on this PR.

dcalvoalonso · 2019-01-30T12:54:39Z

Nice job @jason-fox ! LGTM

fgalan · 2019-01-30T13:00:54Z

I have realized we have other 6 PRs that may suffer the style fixing.

I wouldn't suggest for block the merge of this (some of the other PRs are quite old) but at least lest provide a "migration path" their authors. For instancia:

Add the following devDepencencies to packages.json in your PR branch:
- "prettier": "~1.14.2",
npm install to install the new dependencies.
Run prettier --tab-width 4 --single-quote --trailing-comma es5 --write **/*.js *.js
Check changes (giff diff) and commit them.
Merge master branch into your branch. No style conflic should appear, given both master and your branch would be "in style sync" after step 4. Of course, conflicts may occur, but due to regular causes.

@jason-fox what do you think? Is the procedure complete and precise enough?

jason-fox · 2019-01-30T13:14:35Z

@fgalan - yes that procedure makes sense, especially for small PRs.
However the total will be more than 6 PRs - as I've created prettier PRs for 7 other repos.

fgalan · 2019-01-30T15:53:51Z

Yes... I was meaning 6 in this repo but of course, considered in a wider sense, there are more.

Let's analyze the situation repo by repo. In particular, in this repo we have:

PR [WIP] Extend operators and functions in expressionParser.js #688, authored by me. No problem with adapting it.
PR #671 issue #674 authored by @gabrieledeluca. I think is pretty small, so no problem with it.
PR [WIP] northBound server in HTTPS and Verify ContextBroker Certificate #601 authored by @m4n3dw0lf . I'm also think is pretty small and it shouldn't be a problem.
PR Task/auth support subscriptions #600 authored by @chicco785 . It is somehow large ~340 lines of code. What do you think @chicco785 ? Is it ok to apply the "migration procedure" on it once Add prettier code formatting #753 gets merged?
PR improve https support for subscriptions and updates #594 authored by @chicco785 . It is somehow large ~470 lines of code. What do you think @chicco785 ? Is it ok to apply the "migration procedure" on it once Add prettier code formatting #753 gets merged?
PR Update uuid to version 3.0.0 #526 authored by @marcbachmann. It's small. It shouldn't be a problem.

…prettier

jason-fox · 2019-08-20T11:02:43Z

@fgalan @dcalvoalonso - I have just updated this PR to remove any conflicts from the 2.10.0 codebase. Could you re-review as necessary and merge or close so I no longer have to keep updating the PR?

fgalan · 2019-08-21T14:55:38Z

If this PR is merged other PRs will need adaptation to be merged. Originally we thought that this adaptation would be straightforward (we even define a procedure for that: #753 (comment)). However, that procedure overestimated the difficulties due to exception marks (see telefonicaid/fiware-sth-comet#471 (comment)). This can cause difficulties to other PRs authors and to close PRs with valuable functionality in the worst case. As at then end happened with telefonicaid/fiware-sth-comet#471 (comment)... his author never reopened a new PR for that :(

Taking into account that past experience and the existence of important PRs yet to be merged (as #753 or #800) I'm afraid we cannot merge this one yet. However, I would't opt for closing it, as I think you have done a valuable work here and I would be a pitty lossing it (having said that if you feel more confortable closing this PR or don't want to invest extra effort updating it in the future, it's also fine).

Sorry the inconveniences...

…prettier

jason-fox · 2020-07-15T08:24:24Z

Withdrawn - superseded by #831

#753 prettifies code, #831 does the same job and more (via the eslint prettier plugin.)

jason-fox added 5 commits January 28, 2019 10:48

Add prettier, husky and lint-staged

879f7c3

Run npm run prettier to format files - whitespace changes only.

7ad34bb

Revert Single quote of string due to apostrophe.

913b790

Add exception for single/double quote to pass linter

8fa62e8

* Prettier will always double quote a string containing an apostrophe.

Simplify update

feafc92

fgalan reviewed Jan 28, 2019

View reviewed changes

Husky Update

20bd137

Add prettier to documentation

75c0c54

fgalan reviewed Jan 30, 2019

View reviewed changes

jason-fox added 2 commits January 30, 2019 12:54

Amend tab-width to 4

a4979bd

Re-run prettier across all files.

998f379

fgalan approved these changes Jan 30, 2019

View reviewed changes

Replace ^ with ~ in dependencies

3536dbf

fgalan mentioned this pull request Jan 30, 2019

Add prettier code formatting telefonicaid/perseo-fe#314

Merged

jason-fox added 8 commits January 31, 2019 10:10

Create Prettier Config

73534ef

Rerun prettier - 120 width

74d8ed2

Fix additional linting errors

a3b04be

Re-running prettier with no trailing comma.

77b13e3

Remove package lock

716f799

Merge branch 'master' into feature/prettier and fix conflicts

3167237

Merge branch 'master' into feature/prettier

99d8f1c

Merge commit '86cbf1765676686609265940bfde0c0b3db265e5' into feature/…

9541a14

…prettier

jason-fox added 2 commits June 3, 2019 17:19

Merge branch 'master' into feature/prettier

6ff33a9

Merge branch 'master' into feature/prettier

9e99d91

jason-fox added 4 commits September 19, 2019 10:24

Merge branch 'master' into feature/prettier

f899af4

Merge branch 'master' into feature/prettier

70d677f

Merge commit '5b0ea795c51cb4f903fcbf1853372488380b7f61' into feature/…

d828e9d

…prettier

Merge branch 'master' into feature/prettier

a3c469c

jason-fox mentioned this pull request Nov 26, 2019

Should every IoT Agent align to use eslint #830

Closed

Merge commit 'd47bba79fc5c90c0c0a9d0c88c33c598b8c1ff4c' into feature/…

ccedbdf

…prettier

jason-fox mentioned this pull request Nov 30, 2019

Feature/eslint #831

Merged

jason-fox added 2 commits December 4, 2019 09:22

Merge commit 'd9c5ac9af583e7610eaafa5baa0f4abb1aead5c2' into feature/…

1fb7ed0

…prettier

Merge commit '25b690aa97059ba3e2306f300ca3647486134c18' into feature/…

ad15943

…prettier

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 11, 2020 09:55

Merge branch 'master' into feature/prettier

2335a69

Merge branch 'master' into feature/prettier

d84e426

jason-fox closed this Jul 15, 2020

jason-fox deleted the feature/prettier branch November 20, 2020 14:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add prettier code formatting #753

Add prettier code formatting #753

jason-fox commented Jan 28, 2019

fgalan Jan 28, 2019 •

edited

Loading

jason-fox Jan 29, 2019

fgalan Jan 30, 2019

jason-fox Jan 30, 2019

fgalan Jan 28, 2019

jason-fox Jan 29, 2019

fgalan Jan 30, 2019

fgalan Jan 28, 2019

jason-fox Jan 29, 2019 •

edited

Loading

fgalan Jan 30, 2019 •

edited

Loading

jason-fox Jan 30, 2019

fgalan commented Jan 28, 2019

jason-fox commented Jan 29, 2019 •

edited

Loading

fgalan Jan 30, 2019

jason-fox Jan 30, 2019

fgalan Jan 30, 2019

fgalan left a comment •

edited

Loading

dcalvoalonso commented Jan 30, 2019

fgalan commented Jan 30, 2019 •

edited

Loading

jason-fox commented Jan 30, 2019

fgalan commented Jan 30, 2019 •

edited

Loading

jason-fox commented Aug 20, 2019

fgalan commented Aug 21, 2019

jason-fox commented Jul 15, 2020

Add prettier code formatting #753

Add prettier code formatting #753

Conversation

jason-fox commented Jan 28, 2019

fgalan Jan 28, 2019 • 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

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 Jan 29, 2019 • edited Loading

Choose a reason for hiding this comment

fgalan Jan 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fgalan commented Jan 28, 2019

jason-fox commented Jan 29, 2019 • edited Loading

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 • edited Loading

Choose a reason for hiding this comment

dcalvoalonso commented Jan 30, 2019

fgalan commented Jan 30, 2019 • edited Loading

jason-fox commented Jan 30, 2019

fgalan commented Jan 30, 2019 • edited Loading

jason-fox commented Aug 20, 2019

fgalan commented Aug 21, 2019

jason-fox commented Jul 15, 2020

fgalan Jan 28, 2019 •

edited

Loading

jason-fox Jan 29, 2019 •

edited

Loading

fgalan Jan 30, 2019 •

edited

Loading

jason-fox commented Jan 29, 2019 •

edited

Loading

fgalan left a comment •

edited

Loading

fgalan commented Jan 30, 2019 •

edited

Loading

fgalan commented Jan 30, 2019 •

edited

Loading