Run Nextflow lint in pre-commit

[edmundmiller]

This pull request adds a new linting hook for Nextflow scripts to the pre-commit configuration. This will help ensure consistent formatting and style for the repository's .nf and nextflow.config files.

• The hook is set to run serially due to a known concurrency issue.
• The hook will NOT format the code automatically. It'll just throw an error, and the user will have to correct it by hand.

[robsyme]

Would be good to retain comments if possible.

[ewels]

I think this is a current limitation in the formatter, it can't handle inline comments like this yet.

[bentsherman]

I recommend keeping the formatter off by default. It's not worth losing comments like this one

[edmundmiller]

That's the point of pre-commit though 😞

[maxulysse]

Looking good, but definitively this is WAY too many files modified at once.
Could we batch that in multiple PRs?

[ewels]

I'd much prefer a config file 😬 nextflow-io/nextflow#5934

Though I guess we can always add that later.

[edmundmiller]

I would as well, but perfect is the enemy of good 😉

[bentsherman]

I agree with Maxime. I would turn off the formatting and only fix linter errors in this PR. In a separate PR I would run the formatter as a once-off and undo any comment removals or controversial formatting changes. We should improve the formatter to handle those cases, not the other way around

[edmundmiller]

I agree with Maxime. I would turn off the formatting and only fix linter errors in this PR. In a separate PR I would run the formatter as a once-off and undo any comment removals or controversial formatting changes. We should improve the formatter to handle those cases, not the other way around

Yeah it's a bit much. I'll chat with @nf-core/maintainers about it.

I'm thinking we

1. Turn on the pre-commit formatting(for the CI check and ease)
2. Report any issues we have to https://github.com/nextflow-io/language-server (Or Nextflow proper?)

[ewels]

Works for me 👍🏻 I think a slow roll-out at first until we're confident is a good idea. We can check each module one by one as they are updated to make sure that the changes are sanitised.

This will fail if the locally installed Nextflow version isn't recent enough, right? Any way that we can deal with that gracefully?

[edmundmiller]

This will fail if the locally installed Nextflow version isn't recent enough, right? Any way that we can deal with that gracefully?

How complicated do you want to get?

1. We can use script instead and add a version check that way.

2. We can use the fail directive

-   repo: local
    hooks:
    -   id: changelogs-rst
        name: changelogs must be rst
        entry: changelog filenames must end in .rst
        language: fail
        files: 'changelog/.*(?<!\.rst)$'

3. We can use docker_image or conda to force the right version

[ewels]

How complicated do you want to get?

As little as possible 😅

[maxulysse]

@adamrtalbot played with this too in https://github.com/adamrtalbot/nf-lint-pre-commit

[maxulysse]

Ran it manually for some modules: #8891
It is still slightly messy

[LouisLeNezet]

Hi,
I'm digging up this old PR, as we would like to add nextflow lint to the Github action and therefore it would be nice to also have it to the pre-commit !

[SPPearce]

@bentsherman, @ewels , do you have any progress on improving the formatter?

[maxulysse]

@nf-core-bot fix linting pretty please 🙏

[mashehu]

we need to add setup-nextflow to the linting.yml and fix-litning.yml for this step to work

[ewels]

@bentsherman, @ewels , do you have any progress on improving the formatter?

No updates yet - we know it's a high priority issue and will get to it ASAP, I think it's currently scheduled to be done once the new Records stuff is in place. Hopefully done before the 26.04 release, but might be another month or so until we get to it.

Issues being tracked here, if anything is missing please add more issues.

[SPPearce]

I think that we could move any comments that are going to be affected and apply the auto formatting anyway.

[adamrtalbot]

@adamrtalbot played with this too in https://github.com/adamrtalbot/nf-lint-pre-commit

Updated this here: https://github.com/seqeralabs/nf-lint-pre-commit/releases/tag/v0.2.0

[SPPearce]

Can we also set vscode to do the same on save?

[mashehu]

Can we also set vscode to do the same on save?

this is just the linter without the formatter atm

[jonasscheid]

Can we also set vscode to do the same on save?

I would suggest only doing that if vscode is not in auto-save mode

[mashehu]

My plan for this PR is waiting for @LouisLeNezet to be done with his heroic linting fix efforts and then merge just the pre-commit-config.yml with the nextflow lint hook.

[adamrtalbot]

This is the almost the same as the default for the pre-commit hook: https://github.com/seqeralabs/nf-lint-pre-commit/blob/58b2431ebcdf99a0c024c9c7f695bfa30c01555c/.pre-commit-hooks.yaml#L6

Shall we add *.nextflow files upstream? I've never seen one in the wild!

[mashehu]

Closing in favor of #10241