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.

Sign up for GitHub

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

Jump to bottom

Add Hello-channels, stub for containers etc and reorder training modules #391

Merged
merged 33 commits into from
Oct 22, 2024
Merged

Add Hello-channels, stub for containers etc and reorder training modules #391

merged 33 commits into from
Oct 22, 2024

Conversation

vdauwera
Copy link
Collaborator

  • Split out the joint genotyping from Hello-GATK into Hello-Channels (based on Adam's proposal)
  • Move Hello-Config to earlier

Draft PR: Hello-Channels is a stub

@netlify Netlify
Copy link

netlify bot commented Oct 10, 2024 •
edited
Loading

✅ Deploy Preview for nextflow-training ready!

Name Link
🔨 Latest commit dc14795
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-training/deploys/6718078a0609020008619ac6
😎 Deploy Preview https://deploy-preview-391--nextflow-training.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@vdauwera
Remove GVCF and joint genotyping from Hello-GATK
a742f16 
Simplify Part 2 by making regular VCFs and removing the joint genotyping step.
@vdauwera
Start updating the channels stub
9f4c1d0 
Stub in the direct command invocations, add in switch to GVCF, remove basic GATK steps
@vdauwera
Add introductory explanations
2b4271f 
Added more intro explanations to Hello GATK + equivalent for Hello Channels
@vdauwera
Added joint genotyping to Hello-channels
1048622 
Also improved explanations + tweaked order of steps
@vdauwera
Prettier fixes
08bc0fb
@vdauwera
Merge branch 'master' into gvda-hello-channels
808cccb
@vdauwera
Finish converting the splitText to splitCsv
61f16de 
change filename extension + add .flatten()
@vdauwera
Merge branch 'gvda-hello-channels' of https://github.com/nextflow-io/…
39496f5 
…training into gvda-hello-channels
@vdauwera
Prettier fixes
45f0954 
killing me with the whitespace
This was referenced Oct 17, 2024
Closed
Closed
Open
@vdauwera vdauwera self-assigned this Oct 21, 2024
@vdauwera
Copy link
Collaborator Author

Sticking this here for posterity: I added docker.fixOwnership = true to the nextflow.config to work around this issue: broadinstitute/gatk#8233 (comment)

@kenibrewer kenibrewer changed the base branch from master to dev October 21, 2024 18:08
@kenibrewer
Copy link
Member

I changed the base branch of this PR to dev so we can merge it (along with placeholder content) once the added content is finished.

@vdauwera
Copy link
Collaborator Author

I changed the base branch of this PR to dev so we can merge it (along with placeholder content) once the added content is finished.

Perfect, thank you Ken!

@kenibrewer kenibrewer marked this pull request as ready for review October 21, 2024 22:41
kenibrewer
kenibrewer approved these changes Oct 22, 2024
View reviewed changes
Copy link
Member

@kenibrewer kenibrewer left a comment

Choose a reason for hiding this comment

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

Most of my suggestions are just breaking paragraphs into separate lines for cleaner markdown diffs. When a suggestion involves substantive changes I added a note saying so.

I have a stylistic preference for simpler sentences rather than longer ones. I also use semi-colons only when joining connected thoughts that are relatively short on their own. Feel free to ignore those stylistic suggestions.

There are some outstanding TODO's in this PR, but I would prefer we merge this PR into dev and then tackle the TODOs in a separate branch. That will make it easier for me to merge #405 .

As far as the substantive changes, this seems like an outstanding improvement to our training modules! Marking this approve pending resolution of comments. 🚀

docs/hello_nextflow/02_hello_world.md Outdated Show resolved Hide resolved
docs/hello_nextflow/02_hello_world.md Outdated Show resolved Hide resolved
docs/hello_nextflow/02_hello_world.md Outdated Show resolved Hide resolved
docs/hello_nextflow/02_hello_world.md Outdated Show resolved Hide resolved
docs/hello_nextflow/02_hello_world.md Outdated Show resolved Hide resolved
docs/hello_nextflow/05_hello_channels.md Outdated Show resolved Hide resolved
docs/hello_nextflow/05_hello_channels.md Outdated Show resolved Hide resolved
docs/hello_nextflow/05_hello_channels.md Outdated Show resolved Hide resolved
docs/hello_nextflow/05_hello_channels.md Outdated Show resolved Hide resolved
docs/hello_nextflow/05_hello_channels.md Outdated Show resolved Hide resolved
@vdauwera vdauwera changed the title Add Hello-channels and reorder training modules Add Hello-channels, containers etc and reorder training modules Oct 22, 2024
@vdauwera vdauwera changed the title Add Hello-channels, containers etc and reorder training modules Add Hello-channels, stub for containers etc and reorder training modules Oct 22, 2024
@vdauwera vdauwera merged commit d26900a into dev Oct 22, 2024
8 checks passed
@vdauwera vdauwera deleted the gvda-hello-channels branch October 22, 2024 20:29
@mribeirodantas mribeirodantas mentioned this pull request Oct 27, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxulysse maxulysse maxulysse left review comments

@kenibrewer kenibrewer kenibrewer approved these changes

@mribeirodantas mribeirodantas Awaiting requested review from mribeirodantas

@adamrtalbot adamrtalbot Awaiting requested review from adamrtalbot

Assignees

@vdauwera vdauwera

Labels
enhancement New feature or request gitpod hello series training material
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vdauwera @kenibrewer @maxulysse