-
Notifications
You must be signed in to change notification settings - Fork 74
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
Finish porting the pipeline to updated Nextflow DSL2 syntax #58
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just added a few suggestions. After solving them feel free to merge! :rocket
.github/CONTRIBUTING.md
Outdated
3. Define the output channel if needed (see below). | ||
4. Add any new flags/options to `nextflow.config` with a default (see below). | ||
5. Add any new flags/options to `nextflow_schema.json` with help text (with `nf-core schema build`). | ||
6. Add any new flags/options to the help message (for integer/text parameters, print to help the corresponding `nextflow.config` parameter). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add here add that should be added to the nextflow_schema.json
for consistency with the two previous points?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't understand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Think we can remove it altogether to be honest because it is already mentioned in point 5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in eab36c9
.github/CONTRIBUTING.md
Outdated
6. Add any new flags/options to the help message (for integer/text parameters, print to help the corresponding `nextflow.config` parameter). | ||
7. Add sanity checks for all relevant parameters. | ||
8. Add any new software to the `scrape_software_versions.py` script in `bin/` and the version command to the `scrape_software_versions` process in `main.nf`. | ||
9. Do local tests that the new code works properly and as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like more the previous version:
9. Do local tests that the new code works properly and as expected. | |
9. Perform local tests to validate that the new code works properly and as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got this from the pipeline template so will need to be updated there too. Will fix here for now 👍🏽
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in eab36c9
.github/CONTRIBUTING.md
Outdated
### Software version reporting | ||
|
||
If you add a new tool to the pipeline, please ensure you add the information of the tool to the `get_software_version` process. | ||
|
||
Add to the script block of the process, something like the following: | ||
|
||
```bash | ||
<YOUR_TOOL> --version &> v_<YOUR_TOOL>.txt 2>&1 || true | ||
``` | ||
|
||
or | ||
|
||
```bash | ||
<YOUR_TOOL> --help | head -n 1 &> v_<YOUR_TOOL>.txt 2>&1 || true | ||
``` | ||
|
||
You then need to edit the script `bin/scrape_software_versions.py` to: | ||
|
||
1. Add a Python regex for your tool's `--version` output (as in stored in the `v_<YOUR_TOOL>.txt` file), to ensure the version is reported as a `v` and the version number e.g. `v2.1.1` | ||
2. Add a HTML entry to the `OrderedDict` for formatting in MultiQC. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section is not anymore in the template should we add a software version reporting
section in the template?
### Software version reporting | |
If you add a new tool to the pipeline, please ensure you add the information of the tool to the `get_software_version` process. | |
Add to the script block of the process, something like the following: | |
```bash | |
<YOUR_TOOL> --version &> v_<YOUR_TOOL>.txt 2>&1 || true | |
``` | |
or | |
```bash | |
<YOUR_TOOL> --help | head -n 1 &> v_<YOUR_TOOL>.txt 2>&1 || true | |
``` | |
You then need to edit the script `bin/scrape_software_versions.py` to: | |
1. Add a Python regex for your tool's `--version` output (as in stored in the `v_<YOUR_TOOL>.txt` file), to ensure the version is reported as a `v` and the version number e.g. `v2.1.1` | |
2. Add a HTML entry to the `OrderedDict` for formatting in MultiQC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in eab36c9
No description provided.