+
Skip to content

Conversation

edmundmiller
Copy link
Contributor

Adds pre-commit hooks config for the repo.

Blocked by #3140 currently. Docs if anyone has any ideas https://pre-commit.com/#new-hooks

@edmundmiller edmundmiller self-assigned this Aug 26, 2024
@mashehu
Copy link
Contributor

mashehu commented Aug 26, 2024

I think this should be in the pipeline template, no?

@edmundmiller
Copy link
Contributor Author

They could be in the pipeline template, but I just made a seperate hook that just works for the pipeline and added that to the template.

Might want to look at the stages options. I'm thinking pre-merge-commit and pre-push so it's not too annoying.

edmundmiller added a commit to nf-core/modules that referenced this pull request Nov 20, 2024
edmundmiller added a commit to nf-core/modules that referenced this pull request Nov 20, 2024
edmundmiller added a commit to nf-core/modules that referenced this pull request Nov 21, 2024
* ci: Fix linting getting skipped

* ci: Add list-files json

* ci: Why do we need a Docker container again?

* ci: Can't use a script if it doesn't exist yet

Co-authored-by: adamrtalbot <adamrtalbot@users.noreply.github.com>

* ci: Skip outputs if they're empty

* style: Run lsp on fastqc

* ci: Use files to skip extra steps

* ci(lint): Get the module name

nf-core/tools#3141

* ci: Add what shell to run on

* ci: Clean up the names in the filter step

* ci: Add hack to only lint the real changes

* ci: Add more to debug step

* ci(lint): Get fancy with map

* ci: One more time

* ci: Maybe?

* ci: results => result

* ci: Death by a thousand s's

* ci: Use Set

* ci: Use one at a time

* ci(gpu): Update step name

* ci: Remove clean workspace after run

* ci: Why does that break things?

* Revert "style: Run lsp on fastqc"

This reverts commit da64fce.

---------

Co-authored-by: adamrtalbot <adamrtalbot@users.noreply.github.com>
@edmundmiller edmundmiller marked this pull request as ready for review November 25, 2024 18:44
@edmundmiller
Copy link
Contributor Author

Removed nf-core modules lint for the moment, but I'd like to get this merged for nf-core/modules#7084 to simplify the modules CI 🙏🏻

Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.78%. Comparing base (207129d) to head (7bed55f).
Report is 31 commits behind head on dev.

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit worried that it will be annoying to find linting errors in a subworkflow you are not trying to modify, if we lint all subworkflows.
Otherwise looks good. I have added #3140 to the milestone to work on it

Copy link
Contributor

@mashehu mashehu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commits logging is really bad, therefore I wouldn't add our linting logs to it.

@mirpedrol
Copy link
Member

🧹 spring cleaning message 🌷

What is the status of this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载