RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
316 stars 70 forks source link

Add prettierrc #744

Closed koonpeng closed 3 years ago

koonpeng commented 3 years ago

This adds a .prettierrc.yml file so that prettierrc config file outside the project will not get mistakenly loaded when it goes through husky/lint-staged. I also did a run of prettier and clang-format on the current files, feel free to revert the formatting commits if they are undesired.

Most of the changes are just formatting, only .prettierrc.yml and package.json changes are relevalent.

koonpeng commented 3 years ago

The prettier formatting is conflicting with the eslint settings, even though the eslint config extends eslint-config-prettier, it still defines the incompatible rules so that overwrites them.

Do we use eslint or prettier to perform formatting? And should I change the formatting according to the current eslint or prettier rules?

minggangw commented 3 years ago

Do we use eslint or prettier to perform formatting?

Personally, I prefer prettier.

And should I change the formatting according to the current eslint or prettier rules?

+1 I'd like to format the whole codebase.

I found a article to solve the conflict that I think we could follow https://blog.theodo.com/2019/08/empower-your-dev-environment-with-eslint-prettier-and-editorconfig-with-no-conflicts/

  1. Write .eslintrc.yml with:
    {
    "extends": ["eslint:recommended", "prettier"],
    "env": {
    "es6": true,
    "node": true
    },
    "rules": {
    "prettier/prettier": "error"
    },
    "plugins": [
    "prettier"
    ]
    }
  2. and add eslint-plugin-prettier as devDepenencies. I test locally and it works well :)
minggangw commented 3 years ago

I changed our .eslintrc.yml to

env:
  node: true
  es6: true
globals:
  every: true
  after: true
  constantly: true
ignorePatterns: ["generated/"]
rules:
  prettier/prettier: "error"
extends: ["eslint:recommended"]
plugins: ["prettier"]
parserOptions:
  ecmaVersion: 2018
koonpeng commented 3 years ago

there were some differences between the eslint rules and prettier, for example the eslint rules had max-len: ["error", {"code": 120, "tabWidth": 2, "ignoreComments": true}], iirc prettier's default line length is 80 and we did not change it in args. Should I respect the prettier settings or the eslint rules?

minggangw commented 3 years ago

I suggest we could follow the prettier rules only and remove the current ones, it seems there is always kind of possibility that mixing rules of prettier and eslint leads to conflicts.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.02%) to 91.268% when pulling 896f8ff9832da7a0749e42874acef767ecf7ed81 on add-prettierrc into 1f06987db4e60d0088ec37ee793586e811c5d69c on develop.