Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dummy configs for all missing configs #674

Merged

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Apr 24, 2024

This is for allowing us to uncomment out the part in the pipeline template.

I wonder if it just works 'as is' or if we have to add an explicit profile or not... ideas @maxulysse ?

@jfy133
Copy link
Member Author

jfy133 commented Apr 24, 2024

Not tested because on train

@jfy133 jfy133 requested a review from maxulysse April 24, 2024 19:01
Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

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

LGTM, we just need to be sure we add any new pipeline

@mashehu
Copy link
Contributor

mashehu commented Apr 25, 2024

If you want I can write a github action which periodically checks https://nf-co.re/pipeline_names.json and creates that.

@mashehu
Copy link
Contributor

mashehu commented Apr 25, 2024

One thing I find sub-optimal: we now have conf/pipeline/ and pipeline/. too easy to confuse the two, imo

@maxulysse
Copy link
Member

One thing I find sub-optimal: we now have conf/pipeline/ and pipeline/. too easy to confuse the two, imo

both have different usage, but I see your point

@mashehu
Copy link
Contributor

mashehu commented Apr 25, 2024

yes, different usage and same name is never good

@jfy133
Copy link
Member Author

jfy133 commented Apr 25, 2024

If you want I can write a github action which periodically checks https://nf-co.re/pipeline_names.json and creates that.

That would be awesome

@jfy133
Copy link
Member Author

jfy133 commented Apr 25, 2024

Should I wait for the GHA @mashehu before merging? But otherwise I will prepare the tools template to uncomment the relevant code, and update our new-project-guidance

@mashehu
Copy link
Contributor

mashehu commented Apr 25, 2024

yes, I will add the GHA to this PR. Any chance we can use a different name than pipeline/ for the placeholders?

@jfy133
Copy link
Member Author

jfy133 commented Apr 25, 2024

yes, I will add the GHA to this PR. Any chance we can use a different name than pipeline/ for the placeholders?

No I don't think so. It's already embedded in existing pipelines for a while, so if we changed it would break compatibility with old releases. @maxulysse correct me if I'm wrong

@mashehu
Copy link
Contributor

mashehu commented Apr 25, 2024

okay, GHA is there. as usual need to really test it in production (I ran it locally with act and it looked fine, but we will see). Left some testing code in it, so we will need a clean-up PR after this one is merged and the workflow has been tested

@jfy133
Copy link
Member Author

jfy133 commented Apr 26, 2024

Looks good to me @mashehu !

If you can both review: nf-core/tools#2936 and re-review this one, then i will merge this PR and shortly after the tools one

@jfy133 jfy133 merged commit f4d986c into nf-core:master May 2, 2024
124 checks passed
@jfy133 jfy133 deleted the dummy-templates-for-all-missing--config branch May 2, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants