couchdb-fauxton icon indicating copy to clipboard operation
couchdb-fauxton copied to clipboard

Added editor config based on styleguide - #1205

Open iamgollum opened this issue 4 years ago • 17 comments

Resolves #1205

iamgollum avatar Jul 09 '19 02:07 iamgollum

Should I also add the following? @popojargo and what should be there values

tab_width 3? end_of_line lf? unix? charset trim_trailing_whitespace . true? insert_final_newline . true? max_line_length . 120? (supported by some editors)

iamgollum avatar Jul 09 '19 03:07 iamgollum

  • charset = utf-8
  • trim_trailing_whitespace = true
  • insert_final_new_line = true
  • end_of_line = lf

As for the max_line_length, we suggest line no longer than 120 but its required in some cases...

popojargo avatar Jul 09 '19 03:07 popojargo

@popojargo I am on it ... so you want max_line_length?

iamgollum avatar Jul 09 '19 03:07 iamgollum

@popojargo not sure if I should be force pushing? Are commits squashed on merge and the name of the branch taken as the name of the commit or something to that effect?

iamgollum avatar Jul 09 '19 03:07 iamgollum

This looks cool. What is the value of having this versus setting up prettier that would just run at each commit?

garrensmith avatar Jul 09 '19 08:07 garrensmith

Having prettier running as a commit hook would be a better solution indeed.

popojargo avatar Jul 13 '19 13:07 popojargo

Also, no need to squash on your branch, you can do it at the merge time. Altough, you should always rebase your branch before merging

popojargo avatar Jul 13 '19 13:07 popojargo

@popojargo @garrensmith I think its wise to have both since why would you code in one format and then have it switch to another format on hook. its better to have some consistency no?

Eslint + EditorConfig + Prettier gives you the best of all worlds

EditorConfig is also a safety net if you don't npm install and could be applied in a different context also...

I pushed prettier using husky and pretty-quick. VSCode extension I am using- Prettier - Code formatter however it not aligned with this, because it appears it leverages prettier-eslint which is not supported by pretty-quick: https://github.com/azz/pretty-quick/issues/22

Feel free to play around with it...some attributes I applied for experimentation but there is a discussion on the difference between line length and printWidth.

iamgollum avatar Jul 13 '19 19:07 iamgollum

I figured out how to combine linting with everything else, so I installed lint-staged and applied a regex similar to how you guys did it for the stylecheck script.

However there is still weird behavior with VSCode: https://github.com/prettier/prettier-vscode/issues/371

iamgollum avatar Jul 14 '19 00:07 iamgollum

Did a little experiment, pretty-quick will only trigger once a file has already been previously formatted in prettier. So first time files - unless applying the VSCode prettier extension - will need to be formatted manually. After that, it will work.

I also ensured that in the extension, the flag Prettier: Eslint Integration in the settings. So after playing around with the it, it appears to work just fine.

Unrelated, you could use the precommit hook in npm, but Husky gives more power and configuration.

iamgollum avatar Jul 14 '19 04:07 iamgollum

@popojargo @garrensmith Done with prettier setup. Let me know what you guys think. The one option I was not sure about was the brace stylings ... some files had spaces in the object literals, others did not...

iamgollum avatar Jul 14 '19 14:07 iamgollum

@iamgollum Can you rebase on master?

popojargo avatar Sep 17 '19 20:09 popojargo

@popojargo because the remote branch already exists - rebase wouldn't work right? I would need to merge in this case...

iamgollum avatar Sep 18 '19 01:09 iamgollum

@popojargo applied merge. Hopefully this is good to go ... if not I can roll back and try again with a different strategy and or git message

iamgollum avatar Sep 18 '19 02:09 iamgollum

@iamgollum You need to update the package-lock.json as well. Running npm i should solve that

popojargo avatar Sep 18 '19 02:09 popojargo

@garrensmith @Antonio-Maranhao Any objections to use prettier?

popojargo avatar Sep 18 '19 02:09 popojargo

@iamgollum @popojargo This PR seems stale, any chance to revive it?

wohali avatar Oct 30 '20 19:10 wohali