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

added fc_count_type parameter to biotype_qc command #441

Merged
merged 2 commits into from
Aug 17, 2020
Merged

added fc_count_type parameter to biotype_qc command #441

merged 2 commits into from
Aug 17, 2020

Conversation

pcantalupo
Copy link
Contributor

@pcantalupo pcantalupo commented Jul 14, 2020

Many thanks to contributing to nf-core/rnaseq!

To ensure that your build passes, please make sure your pull request is to the dev branch rather than to master. Thank you!

Please fill in the appropriate checklist below (delete whatever is not relevant). These are the most common things requested on pull requests (PRs).

PR checklist

  • PR is to dev rather than master
  • This comment contains a description of changes (with reason)
  • If you've fixed a bug or added code that should be tested, add tests!
  • If necessary, also make a PR on the nf-core/rnaseq branch on the nf-core/test-datasets repo
  • Ensure the test suite passes (nextflow run . -profile test,docker).
  • Make sure your code lints (nf-core lint .).
  • Documentation in docs is updated
  • CHANGELOG.md is updated
  • README.md is updated

Learn more about contributing: https://github.com/nf-core/rnaseq/tree/master/.github/CONTRIBUTING.md

@apeltzer
Copy link
Member

Please also update CHANGELOG :-)

@apeltzer
Copy link
Member

x-ref #440

Copy link
Member

@apeltzer apeltzer left a comment

Choose a reason for hiding this comment

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

Please update CHANGELOG.md in the FIXED section 👍

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

  • params.fc_count_type needs to be initialised in nextflow.config
  • If not set (eg. remains as default null) then we shouldn't include it in the command, as we will get -t null in the command

@pcantalupo
Copy link
Contributor Author

* `params.fc_count_type` needs to be initialised in `nextflow.config`

* If not set (eg. remains as default `null`) then we shouldn't include it in the command, as we will get `-t null` in the command

Just checked...it is already set to a default value of 'exon': https://github.com/nf-core/rnaseq/blob/master/nextflow.config#L58

@ewels
Copy link
Member

ewels commented Jul 14, 2020

Ah I'm sorry - I only looked at the diff and assumed that you were adding a totally new parameter. Please ignore both of my comments then 👍

@apeltzer apeltzer changed the base branch from dev to make-1-4-3-release August 17, 2020 11:44
@apeltzer
Copy link
Member

Thank you @pcantalupo - I'm merging this to the make-1-4-3-release branch now and will updatte it accordingly :-)

@apeltzer apeltzer merged commit b22883e into nf-core:make-1-4-3-release Aug 17, 2020
@pcantalupo pcantalupo deleted the issue440 branch August 17, 2020 11:49
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