Use prettier #1470

ewels · 2022-03-17T15:36:32Z

PR checklist

This comment contains a description of changes (with reason)
CHANGELOG.md is updated
If you've fixed a bug or added code that should be tested, add tests!
Documentation in docs is updated

edmundmiller

Looks like a great start!

ewels · 2022-03-17T16:10:29Z

I tried having 2 space indentation for everything, but 4 spaces for markdown files:

printWidth: 120
tabWidth: 2
overrides:
  - files:
      - "*.md"
    options:
      tabWidth: 4

But Prettier apparently has a weird bug where doing this results in markdown lists with indentation after the bullet:

-   Updated `nf-core download` to work with latest DSL2 syntax for containers ([#1379](https://github.com/nf-core/tools/issues/1379))
-   Made `nf-core modules create` detect repository type with explicit `.nf-core.yml` instead of random readme stuff ([#1391](https://github.com/nf-core/tools/pull/1391))
-   Added a Gitpod environment and Dockerfile ([#1384](https://github.com/nf-core/tools/pull/1384))
    -   Adds conda, Nextflow, nf-core, pytest-workflow, mamba, and pip to base Gitpod Docker image.

should be:

- Updated `nf-core download` to work with latest DSL2 syntax for containers ([#1379](https://github.com/nf-core/tools/issues/1379))
- Made `nf-core modules create` detect repository type with explicit `.nf-core.yml` instead of random readme stuff ([#1391](https://github.com/nf-core/tools/pull/1391))
- Added a Gitpod environment and Dockerfile ([#1384](https://github.com/nf-core/tools/pull/1384))
    - Adds conda, Nextflow, nf-core, pytest-workflow, mamba, and pip to base Gitpod Docker image.

This is discussed in prettier/prettier#5019 but is not yet resolved.

Anyway, I can't remember why we wanted 4-space indentation in markdown now? I think it was GitHub rendering, but I just checked this PR and it seems to render fine. I found some GitHub issues suggesting that this used to be a problem on GitHub but since they changed markdown renderers (years ago now) it's not. So maybe this fixed itself?

Anyway, TLDR is that everything is 2-space indentation now, including markdown. Hopefully this doesn't break too much stuff.

ewels · 2022-03-17T18:06:05Z

I don't understand why the CI tests can't find the repository type 🤔

ewels · 2022-03-17T18:15:20Z

ohh it's because it's not within the pipeline repo 🤦🏻

Ok then we should probably also make the error message more obvious here 😅

…ompile

This means that we can run prettier directly on the entire codebase, without ignoring the template. Will probably need quite a lot of sanity checking to make sure that I didn't break stuff!

ewels · 2022-03-17T23:34:24Z

Right, that'll do for tonight.

Still want to do a sanity check that the output from the pipeline template is basically (functionally) still the same and that I haven't accidentally broken anything in the GitHub actions workflows for example. But basically ready for review and testing now I think....??

nf-core#1470 (comment)

nf-core/tools#1470 (comment)

ewels · 2022-03-18T09:53:58Z

Ok, updated to keep 4-space indentation for JSON files to make the diff as small as possible so that template merges are not a nightmare.

Also fixed some other linting errors in created pipelines.

ggabernet · 2022-03-18T09:59:43Z

nf_core/module-template/modules/meta.yml

@@ -30,13 +30,13 @@ input:

 ## TODO nf-core: Add a description of all of the variables used as output
 output:
-  {% if has_meta -%}
+  #{% if has_meta -%} Only when we have meta


was commenting the line here intended?

Yes - Prettier breaks if it can't parse the YAML files. Sticking the Jinja templating stuff either in quotes or on comment lines means that the template is still valid YAML and so prettier can format it.

Note that this is a YAML comment and Jinja ignores it. If we wanted a Jinja comment that's done with {# jinja here #}. So the YAML comment # here has no effect.

https://prettier.io/docs/en/ignore.html#yaml
I think this is what we need.

ggabernet · 2022-03-18T10:02:55Z

The changes due to the new linter look good to me, not too many changes are needed!

apeltzer

Had to partially re-review after starting yesterday already and some changes came in later 👍🏻 Looks very good to me, lets hope its not too much effort when the template updates are coming in ;-)

* style: Add prettier config files * build: Add prettier vscode extension * ci: Replace markdownlint and yamllint with prettier * style: Run prettier * style: Use indent of 2 for markdown as well nf-core/tools#1470 (comment) * style: Fix indent * style: Let editorconfig take over tab widths * style: yaml => yml * ci: Run prettier once Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se> Co-authored-by: Phil Ewels <phil.ewels@scilifelab.se>

ewels added 4 commits March 17, 2022 16:17

Add Prettier, update actions linting workflows

66b1409

Ignore built API docs

2584cbd

Ignore the templates because of jinja2 tags

0d0a7e4

Actions CI: add a module to the empty template pipeline

7dbe68c

edmundmiller self-requested a review March 17, 2022 15:37

edmundmiller assigned ewels Mar 17, 2022

edmundmiller approved these changes Mar 17, 2022

View reviewed changes

This comment was marked as resolved.

Sign in to view

ewels added 2 commits March 17, 2022 17:05

Remove config files for markdownlint and yamllint

0c0aac4

Run prettier

5f80b6f

Nicer workflow name

0786955

ewels added 2 commits March 17, 2022 19:09

Resolve merge conflicts

48b58d9

Prettier: ignore testing directory

81bf944

ewels added 14 commits March 17, 2022 19:17

Run new compiled pipeline linting commands in the correct directory

756ffeb

Add .prettierrc.yaml to the pipeline-template

847ca2a

Remove yamllint and markdownlint configs from the pipeline template

23d33ca

Run prettier on the pipeline template folder

73f4ff2

Manually fix remaining prettier errors in template files that won't c…

a53ebfb

…ompile

Template: Remove old linting, add Prettier linting CI

181c9bb

Try to move templating logic to make YAML files valid.

8e6bbd8

This means that we can run prettier directly on the entire codebase, without ignoring the template. Will probably need quite a lot of sanity checking to make sure that I didn't break stuff!

Simpler workaround for CI tests by just using YAML quotes

70c27cc

Work around Prettier auto-formatting in pipeline readme

5736a05

Trailing whitespace after jinja2 tags

861555b

Update nf-core pipeline lint tests

6acf5d5

yaml/yml typo

9a81137

.prettierrc yaml -> yml

cfb217f

Update modules before running nf-core lint

fb5211a

ewels force-pushed the use-prettier branch from 32ff872 to fb5211a Compare March 17, 2022 23:02

Don't test pipeline template with black

3691805

ewels force-pushed the use-prettier branch from 2162301 to 0670849 Compare March 17, 2022 23:15

Move sync test to before we change anything

6e60a8d

ewels force-pushed the use-prettier branch from 3cc89d3 to 6e60a8d Compare March 17, 2022 23:22

ewels added 2 commits March 18, 2022 00:24

YAML quotes for prettier + jinja parsing

04a3b8e

Fix obscure bug that was breaking pytest

7841b4a

ewels added the WIP Work in progress label Mar 17, 2022

ewels marked this pull request as draft March 17, 2022 23:32

edmundmiller added 2 commits March 17, 2022 19:43

build(gitpod): Swap markdownlint for prettier

bf9e7af

style(editorconfig): Use 2 spaces for markdown

207d7c6

nf-core#1470 (comment)

edmundmiller added a commit to nf-core/modules that referenced this pull request Mar 18, 2022

style: Use indent of 2 for markdown as well

990264b

nf-core/tools#1470 (comment)

ewels removed the WIP Work in progress label Mar 18, 2022

ewels marked this pull request as ready for review March 18, 2022 08:29

ewels linked an issue Mar 18, 2022 that may be closed by this pull request

Rewrite YAML + Markdown linting, use Prettier #1457

Closed

ewels added 4 commits March 18, 2022 10:42

Change indentation rules to minimise diffs

24e022c

Reformat JSON to 4 spaces with prettier

d668dd2

Update custom/dumpsoftwareversions module

1405d48

Fix new MultiQC linting for report comment

3d07d69

ggabernet reviewed Mar 18, 2022

View reviewed changes

Use .editorconfig for indentation config

06275d1

apeltzer approved these changes Mar 18, 2022

View reviewed changes

Add a default for params.outdir on the test config

a6dbdb1

ewels merged commit 3abb7ef into nf-core:dev Mar 18, 2022

ewels deleted the use-prettier branch March 18, 2022 10:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use prettier #1470

Use prettier #1470

ewels commented Mar 17, 2022

edmundmiller left a comment

This comment was marked as resolved.

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 18, 2022

ggabernet Mar 18, 2022

ewels Mar 18, 2022

ewels Mar 18, 2022

edmundmiller Mar 18, 2022

ggabernet commented Mar 18, 2022

apeltzer left a comment

Use prettier #1470

Use prettier #1470

Conversation

ewels commented Mar 17, 2022

PR checklist

edmundmiller left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 17, 2022

ewels commented Mar 18, 2022

ggabernet Mar 18, 2022

Choose a reason for hiding this comment

ewels Mar 18, 2022

Choose a reason for hiding this comment

ewels Mar 18, 2022

Choose a reason for hiding this comment

edmundmiller Mar 18, 2022

Choose a reason for hiding this comment

ggabernet commented Mar 18, 2022

apeltzer left a comment

Choose a reason for hiding this comment