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

Update README.md #4

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Update README.md #4

wants to merge 1 commit into from

Conversation

aofarrel
Copy link

  • Remove Travis CI shield
  • Replace Docker command with a better one
  • Remove non-functional Dockstore CLI instructions (I did some tinkering to see if I could get this to work but it seems the CWL is out of date, and it doesn't like the WDL either)

Would be willing to revert the deletion of of the Dockstore CLI stuff once the CWL is fixed.

@aofarrel aofarrel requested review from coverbeck and denis-yuen June 15, 2022 23:26
@aofarrel aofarrel self-assigned this Jun 15, 2022
@aofarrel aofarrel force-pushed the update-dockerfile branch from 25dd87c to 82788a9 Compare June 15, 2022 23:30
Copy link
Member

@denis-yuen denis-yuen left a comment

Choose a reason for hiding this comment

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

I may be missing context, is this used in our tutorials?
Gathering info to prioritize

@aofarrel
Copy link
Author

aofarrel commented Jun 16, 2022

Yes, this is the repo used in Getting Started with CWL and Getting Started with Nextflow. Due to the issues with this repo, I previously modified Getting Started with WDL to link to dockstore/bamstats-wdl instead. Ideally, we'd clean up this repo (which will take more than just a readme adjustment) and sunset dockstore/bamstats-wdl.

@aofarrel aofarrel requested a review from denis-yuen June 16, 2022 15:05
Copy link
Member

@denis-yuen denis-yuen left a comment

Choose a reason for hiding this comment

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

Ok, created https://ucsc-cgl.atlassian.net/browse/SEAB-4500
Think it makes more sense to investigate if there is an issue with the CLI than remove if it is a part of the live tutorial

Base automatically changed from update-dockerfile to develop June 16, 2022 15:18
Copy link

@coverbeck coverbeck left a comment

Choose a reason for hiding this comment

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

Was here, commenting so it doesn't show up in my review bucket (I think we should resolve the CLI issue that Denis points out first).

@amarjandu
Copy link

@aofarrel it looks like the version of CWL that the quay.io has is draft-3 and the cwl-runner that ds-cli uses dropped support for draft-3. 3672ac0 should have updated the versioning of the cwl file to be on v1.0.

When running this command it appears that I get the old version of the CWL file.

dockstore tool cwl --entry quay.io/collaboratory/dockstore-tool-bamstats:1.25:develop

Is there a way to update the version of the file, would we have to cut a new release on quay.io?

@denis-yuen
Copy link
Member

Is there a way to update the version of the file, would we have to cut a new release on quay.io?

Yeah, that said. I think unless the build trigger has broken, cutting a new release on github should create a new tag on quay.io

@amarjandu
Copy link

@aofarrel I added a PR to fix the tutorial. #5

@denis-yuen denis-yuen self-requested a review June 29, 2022 21:14
Copy link
Member

@denis-yuen denis-yuen left a comment

Choose a reason for hiding this comment

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

Was here, commenting so it doesn't show up in my review bucket (I think we should resolve the CLI issue that Denis points out first).

ditto

@denis-yuen denis-yuen mentioned this pull request Aug 9, 2022
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.

4 participants