search

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

Update episode 5 and 6 #56

Closed ALuesink closed 2 years ago

ALuesink commented 2 years ago

Still in progress

netlify[bot] commented 2 years ago

✔️ Add Deploy Preview notifications as pull request comments when Deploy Preview succeeds

🔨 Explore the source changes: 3c77e8a4dcada7fdc6920f10bd16822a0194391f

🔍 Inspect the deploy log: https://app.netlify.com/sites/thirsty-hoover-1e0704/deploys/61f170f2542d590007ee9ea8

😎 Browse the preview: https://deploy-preview-56--thirsty-hoover-1e0704.netlify.app

ALuesink commented 2 years ago

Episode 5 - Documentation and Citation is migrated to the intermediate tutorial (https://github.com/common-workflow-lab/cwl-intermediate-tutorial/pull/7)

ALuesink commented 2 years ago

I've added a new episode at the end called More information with links to useful websites

mr-c commented 2 years ago

@gcapes Thanks for all your assistance! Can you review this PR? Since it is bigger I want to finish it before reviewing yours

gcapes commented 2 years ago

Sure - I'll start on it tomorrow. FWIW my PRs focus on the earlier episodes so could be reviewed independently of this one.

gcapes commented 2 years ago

I've now fixed all the suggestions, except I can't reproduce the error from "Using tabs instead of spaces".

I also seem unable to push to this PR's branch, so I'll make a new PR to replace this one.

mr-c commented 2 years ago

I've now fixed all the suggestions, except I can't reproduce the error from "Using tabs instead of spaces".

I also seem unable to push to this PR's branch, so I'll make a new PR to replace this one.

Did you try gh pr checkout 56 using https://cli.github.com/ ?

gcapes commented 2 years ago

No, I've not used the github cli, but that command would only check out the branch (wouldn't it?), which I've already done. I've locally rebased onto gh-pages, and made the fixes.

mr-c commented 2 years ago

No, I've not used the github cli, but that command would only check out the branch (wouldn't it?), which I've already done. I've locally rebased onto gh-pages, and made the fixes.

It sets up the remote to push as well. Anyhow, next time

gcapes commented 2 years ago

Look like I just figured it out :) I'd renamed my local branch, and it looks like that has to match the remote branch name.

gcapes commented 2 years ago

Can someone suggest how to reproduce the tabs instead of spaces error?

mr-c commented 2 years ago

@ALuesink do you still have the error.cwl file?

ALuesink commented 2 years ago

@ALuesink do you still have the error.cwl file?

Yes, I have it. However, I now see it has no tabs so it would not create the error. I've tried a file with tabs instead of spaces, but I don't get the found character \t error. This time I get 'NoneType' object has no attribute .... Is it possible the error has changed in the meantime?

gcapes commented 2 years ago

Thanks for checking. I get the same error I when trying to reproduce an example with tabs. I'll add an example to this episode next week.

Thanks Gerard

Gerard Capes Research Applications, IT Services, University Of Manchester Hours: Tues - Fri, 08:00 - 16:00

From: ALuesink @.> Sent: 21 January 2022 16:47 To: carpentries-incubator/cwl-novice-tutorial @.> Cc: Gerard Capes @.>; Mention @.> Subject: Re: [carpentries-incubator/cwl-novice-tutorial] Update episode 5 and 6 (PR #56)

@ALuesinkhttps://github.com/ALuesink do you still have the error.cwl file?

Yes, I have it. However, I now see it has no tabs so it would not create the error. I've tried a file with tabs instead of spaces, but I don't get the found character \t error. This time I get 'NoneType' object has no attribute .... Is it possible the error has changed in the meantime?

— Reply to this email directly, view it on GitHubhttps://github.com/carpentries-incubator/cwl-novice-tutorial/pull/56#issuecomment-1018680133, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC66UPXFY2E73J7X6VLR253UXGE2VANCNFSM5JHVK57A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub. You are receiving this because you were mentioned.Message ID: @.***>

gcapes commented 2 years ago

This is now ready for review/merge. The 'wiring' error I haven't done, because I can't actually run the workflow in this lesson. It crashes my laptop part way through.

I would think these would work well as exercises rather than examples, but have followed the pattern in the original submission for now.