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

address reviewer comments for first release #156

Merged
merged 42 commits into from
Nov 17, 2024

Conversation

atrigila
Copy link
Collaborator

@atrigila atrigila commented Nov 5, 2024

Address @mashehu's (thank you!!) comments from first release PR.

  • Change version of nf-test
  • Use setup-nf-test instead
  • Update date to release date
  • Change Figures in tables to Figures+captions
  • Add file titles to Readme.md: such as ```csv title="samplesheet.csv"
  • Replace table with figures for ordered list
  • Order tools alphabetically based on the tool name
  • Check assets/chr_rename_del.txt for chrX X
  • Move info in DEVELOPMENT.md to CONTRIBUTING.md. Remove DEVELOPMENT.md.
  • Check docs/images/Logo.svg
  • Add a light-gray background to figures, otherwise the black will blend in with the background on the website in dark mode
  • Fix typo in docs/images/metro/metro.py
  • Check if docs/images/metro/txt2image.md is required
  • Fix typos in output.md: "steps".
  • Use one line per sentence in markdown. Makes the diffs a bit nicer to read.
  • Explain in output.md where are tsv files located.
  • Ensure consistency in VCF/vcf writing across output.md and README.md.
  • Remove duplicate in MULTIQC in output.md
  • Have three samples in usage.md instead of 6.
  • Check grammar and typos in usage.md.
  • Remove info described several times in usage.md.
  • Give each of the tools a higher level heading, so that they appear on the ToC on the right, which might make it easier to navigate.
  • Add groovy in Custom Tool Arguments section
  • Remove modules/local/*/tags.yml
  • Check indentation in nextflow.config
  • Remove pattern checking in boolean values (rename_chr and compute_freq)
  • Add a pattern checking for a comma separated list (ask for clarification on this requirement)
  • Remove pattern in maximal number of individuals per batch for imputation.
  • Add 0 in Minimum genotype likelihood probability P(G|R) in validation data.

@atrigila atrigila changed the title update nf-test version address reviewer comments for first release Nov 5, 2024
@atrigila
Copy link
Collaborator Author

atrigila commented Nov 5, 2024

I will continue addressing these changes tomorrow

@atrigila atrigila marked this pull request as ready for review November 10, 2024 16:57
@atrigila
Copy link
Collaborator Author

The update to release date will be done once all changes are merged and reviewers approve the final release PR

CITATIONS.md Outdated Show resolved Hide resolved
docs/images/metro/txt2image.md Show resolved Hide resolved
docs/images/metro/phaseimpute.drawio.png Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@LouisLeNezet LouisLeNezet left a comment

Choose a reason for hiding this comment

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

The metro map is really nice to understand which tools are used when.
We can improve it in the future if we want.

docs/images/metro/phaseimpute.drawio.png Outdated Show resolved Hide resolved
@atrigila atrigila merged commit da2d16b into nf-core:dev Nov 17, 2024
11 checks passed
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.

2 participants