Dvc.org: pre-commit hook prettier (vs npx prettier) questions

Created on 20 May 2019  ·  8Comments  ·  Source: iterative/dvc.org

First, running npm run prettier-md does not yield the same results as what the pre-commit hook does (hook installation explained in https://dvc.org/doc/user-guide/contributing-documentation). Should it? And how do we equalize them if so?

For example with npm the bullet lists are standardized to * while with the pre-commit hook they're -. I understand we want the latter.

UPDATE: I needed to run npm install as my node_modules prettier version was outdated. npx prettier --write <file pattern>. Apparently oesn't necessarily behave the same as the pre-commit hook (which specifies a prettier version in .pre-commit-config.yaml) so

  • [x] I think we should remind people in the contribution guides to npm i often.

Secondly, the pre-commit hook introduces extra indentation in YAML block e.g.
```yaml
md5: df06d8d51e6483ed5a74d3979f8fe42e
outs:
-- cache: true

  • md5: b8f4d5a78e55e88906d5f4aeaf43802e.dir
  • metric: false
  • path: pics

    • cache: true

  • md5: b8f4d5a78e55e88906d5f4aeaf43802e.dir
  • metric: false
  • path: pics
    wdir: .
    ```
    (from .../commands-reference/add.md) but those are just copy-paste of actual DVC-files so

  • [x] Last, we need to run prettier once we're sure how to and that it's behaving correctly on all .md files with
$ npm run prettier-md
bug doc-content

All 8 comments

On question 1, from https://discordapp.com/channels/485586884165107732/565759186693128202/580164996198301696

...how will the configurations of pre-commit hook and running prettier from command line match(?)
their version will always need to be in sync. Currently my prettier version is 1.17.1. So in my YAML, I changed it to 1.17.1 so they both match.

So should we then establish a strict version of prettier in the package.json file? It currently is"^1.17.0"

@algomaster99 would you like to briefly mention the option to use husky here? Thanks

"husky": {
  "hooks": {
    "pre-commit": "lint-staged" 
    },
},
"lint-staged": {
  "static/docs/**/*.md": [ "prettier --write"],
}

This is one example of configuring pre commit hook using husky and lint-staged. Husky gets triggered before committing and runs lint-staged. There are more options which can be found in https://prettier.io/docs/en/precommit.html.
Lint-staged will respect our .prettierrc file as well. Another way is using pretty-quick. It's more or less same and it also follows .prettierrc, .prettierignore, etc.

If husky isn't used and we are sticking with pre-commit, then we will have to always keep a check if versions of both are same.

@algomaster99 Looks like husky is a good option to keep everything under package.json control. I like it. 👍 After that we can even enable a JS linter with it as far as I understand.

@jorgeorpinel I agree that we should keep the version same. Allow patches, but not major changes.

OK, I changed it to ~1.17.0 in package.json for now (only patches, no minor or major version updates).

I think we should definitely remove the pre-commit Python module because its confusing to have prettier installed as an npm devDependency on one hand, and by pre-commit separately for the Git hook. Whatever applies the git pre commit hook should be able to re-use the prettier node module.

In fact do we use the Python virtualenv for anything else? I think it'd be best if we didn't mix Python into this repo if not needed as this is a js app (with static md files).

I see you already started a new issue on this (: https://github.com/iterative/dvc.org/issues/372 – I'll summarize my comments over there.

@algomaster99 but before that, what do you think about the 2nd checkbox in this issue's description? (Configuring the linter so it doesn't mess with YAML code blocks.) Since you're already familiar with prettier is that something you think you could help with?

I can do the 3rd checkbox (basically npm run prettier-md) once that is fixed, or feel fee to throw it in too. Please just lmk either way in order to figure this out myself if you're unavailable. Thanks

@jorgeorpinel I am not near my computer right now. But I have experimented with YAML code blocks. prettier automatically gives one space indentation between hypen and the value. It's not very strict though. You can try it out here. I'll try to help you with this as soon as possible. 😁

I'll take care of this since you're working on the other related issue. Thanks @algomaster99 !

Was this page helpful?
0 / 5 - 0 ratings