-
Notifications
You must be signed in to change notification settings - Fork 138
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
Hello-seqera #413
Hello-seqera #413
Conversation
✅ Deploy Preview for nextflow-training ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
540bf00
to
6fc59eb
Compare
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.
A few early comments, apologies if you weren't looking for review just yet, @vdauwera pointed me in this direction ;-)
6c172f3
to
7ef3682
Compare
Thanks @pinin4fjords for the early comments. Unfortunately, many of the comments related to things that I had just copied over from your guide and planned to rework. The part of the PR that is now ready to review is Part 1 in I sketched out the outline of what I plan to do for Parts 2-4 in the following files:
I'm about to head to the airport, but if you have time to review part 1 and/or open a PR into this branch that moves content from |
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.
Comments on part 1, just a suggestion about how the config stuff is described.
Boarding plane now, will try more later if I can :-)
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 is ready for review. I decided to stop after the first two sections as the "simulated slurm tower agent" idea I had for a hands-on teaching of compute environments just didn't seem right once I wrote it up. (It's stashed in #420 ).
For now, let's get these two sections in and come back later to that idea after we figure out how to run slum, grid engine or some other executor within a devcontainer.
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 like the flow a lot, feels just right in terms of continuity with the previous parts. There are a few explanations that I felt could be clarified a little but I'm totally ok leaving that for an ulterior round. I didn't test it in practice yet but (what could possibly go wrong) it all seems really solid. Big thumbs up from me.
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.
Great work, Ken! I left a few suggestions/comments.
📝 docs(seqera/run_with_cli): incorporate feedback from review
⚰️ dead(hello_seqera): remove unused portions of basic training
0a966f4
to
b73f9b5
Compare
Co-authored by: Geraldine Van der Auwera <[email protected]> Co-authored by: Marcel Ribeiro-Dantas <[email protected]>
b73f9b5
to
a917fe7
Compare
This adds the first two sections of
hello_seqera
a training module intended to provide a hands-on approach to learning Nextflow.The main portions covered are:
-with-tower
.I had intended to include more content including a simulated Compute Environment setup, but I decided that approach needs more work (specifically we need to figure which of the HPC executors we can run from within a Devcontainer). That partial work has been moved into #420.