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

Prepare for release 1.0.0 "Naked Chicken" #11

Merged
merged 13 commits into from
Nov 27, 2018
Merged

Conversation

apeltzer
Copy link
Member

This will ultimately be the first release of nf-core/openmspeptidequant, "Naked Chicken".

PR checklist

  • 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/openmspeptidequant 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/openmspeptidequant/tree/master/.github/CONTRIBUTING.md

@ewels
Copy link
Member

ewels commented Nov 20, 2018

Naked Chicken - really?? 😆

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.

Few minor things:

  • main.nf is missing ASCII art for the help text
  • nextflow.config has the standard profile twice
  • Most of the configs in base.config could be grouped into a couple of withLabel statements
  • Would be nice to harmonise the profiles a bit with the base template (add uppmax, add zurich to the template)
  • Nextflow throws a warning:
    • WARN: The operator `first` is useless when applied to a value channel which returns a single value by definition -- check channel `id_files_merged_psm_perc_filtered`
      
  • Strange minor markdown rendering bug in docs:
    image
  • num_threads is documented (and here) and in nextflow.config but not used in main.nf
  • Docs don't describe several profiles

Generally looking really good though!

Phil

@apeltzer
Copy link
Member Author

Naked Chicken - really?? 😆

Yes, @Leon-Bichmann had the idea, not my choice :-D

@apeltzer
Copy link
Member Author

All points mentioned by @ewels addressed now:

  • ASCII Art added
  • got rid of the first() statement which was unnecessary
  • Added documentation fixes for markdown rendering issues
  • Added Docs for Profiles present atm (others will come via Sync)
  • Submitted separate PR to tools for having more profiles there (e.g. UZH, also introduced sorting there in the PR)
  • Removed redundant resource statements in the base.config, these are now defaults
    • Only resource requests bigger or smaller get their own labels now
  • Got rid of num_threadswhich isn't used at all in the pipeline anymore

Copy link
Collaborator

@Leon-Bichmann Leon-Bichmann left a comment

Choose a reason for hiding this comment

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

Currently I dont produce any qc analysis and no output visualizations.
I will probably add some in a later release.

docs/usage.md Show resolved Hide resolved
@apeltzer apeltzer changed the title Prepare for release 1.0 "Naked Chicken" Prepare for release 1.0.0 "Naked Chicken" Nov 22, 2018
@apeltzer
Copy link
Member Author

Complete rename solved :-)

@sven1103
Copy link
Member

The openms version in the environment spec must be openms=2.4.0

@apeltzer
Copy link
Member Author

Had to adapt the test-datasets first, now this works.

@apeltzer
Copy link
Member Author

I guess the chicken has landed :-)

@Leon-Bichmann
Copy link
Collaborator

anything missing still for the chicken to leave the nest?

@apeltzer
Copy link
Member Author

A review by @ewels for example ;-)

@apeltzer
Copy link
Member Author

I think this is fine, will merge to dev atm

@apeltzer apeltzer merged commit e3a5e28 into nf-core:dev Nov 27, 2018
Leon-Bichmann added a commit that referenced this pull request Feb 28, 2019
marissaDubbelaar added a commit that referenced this pull request Apr 5, 2022
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.

None yet

4 participants