carpentries-incubator / cwl-novice-tutorial

Introduction to Workflows with Common Workflow Language
https://carpentries-incubator.github.io/cwl-novice-tutorial/
Other
11 stars 20 forks source link

Script episode #20

Closed skanwal closed 3 years ago

skanwal commented 3 years ago

Initial Pull Request to initiate discussion on this episode

Work done so far:

Since, I haven't attended any meetings/discussions wrt to the lesson development - it would be helpful to have feedback on this work to confirm I am heading in the right direction. Another point I wanted to have input was if we should get rid of stdout and just focus on capturing script?

Happy to take inputs or suggestions. I have followed through other PRs and hope I am following correct format. It'll be helpful to have confirmation if the format/layout is correct or any changes are needed. Thanks.

skanwal commented 3 years ago

Hello and happy new year :)

Thanks @tobyhodges and @mr-c for the feedback and comments. Together with the resources Toby shared in his comment, I have a much better understanding of purpose and format of these exercises.

This is a bit difficult concept to teach to novices but helped me greatly with brainstorming and coming-up with simpler yet conceptually rich exercises and solutions.

I have tried best to address all comments, feedback is welcomed. Also, happy to make further modifications, if needed.

tobyhodges commented 3 years ago

Hey @skanwal and Happy New Year! I haven't had time to devote to this yet but I haven't forgotten about it! I hope to be able to review the latest changes tomorrow.

skanwal commented 3 years ago

@tobyhodges - thanks much for the review and feedback! Glad to know it fits better with the objectives and format. I have wrapped code blocks in ~~~.

I hope I have correctly wrapped the Solution sections for each exercise. Please let me know if the format is incorrect.

skanwal commented 3 years ago

@tobyhodges

Only one comment I need you to address, regarding an inconsistency between the code blocks in an exercise and its solution.

Is this referring to lines spaces between exercise and its solution? If you can please point one of these, I can fix rest of the inconsistencies as well.

tobyhodges commented 3 years ago

@tobyhodges

Only one comment I need you to address, regarding an inconsistency between the code blocks in an exercise and its solution.

Is this referring to lines spaces between exercise and its solution? If you can please point one of these, I can fix rest of the inconsistencies as well.

The inconsistency between the code block in the exercise and the code block in the solution is what I was referring to:

These lines are not present in the code block for the exercise, above. Should they be removed here? Or added in the previous block?

skanwal commented 3 years ago

@tobyhodges - please let me know if you're happy with the state of this exercise and is ready to be merged.

Thanks.

tobyhodges commented 3 years ago

@mr-c please take another look. I think this is ready to go but GitHub wants your approval too before I merge the PR

skanwal commented 3 years ago

@mr-c - as per your suggestion, I have updated the runtime script exercise under this commit.

Please let me know if further changes are required.

tobyhodges commented 3 years ago

🎉 great to see this get merged! Thanks for all your work on this @skanwal