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 supported workflow language versions to README #4923

Merged
merged 10 commits into from
May 21, 2024
Merged

Add supported workflow language versions to README #4923

merged 10 commits into from
May 21, 2024

Conversation

adamnovak
Copy link
Member

This will fix #4896.

Changelog Entry

To be copied to the draft changelog by merger:

  • Toil README now advertises supported workflow language versions

Reviewer Checklist

  • Make sure it is coming from issues/XXXX-fix-the-thing in the Toil repo, or from an external repo.
    • If it is coming from an external repo, make sure to pull it in for CI with: 
      contrib/admin/test-pr otheruser theirbranchname issues/XXXX-fix-the-thing
    • If there is no associated issue, create one.
  • Read through the code changes. Make sure that it doesn't have:
    • Addition of trailing whitespace.
    • New variable or member names in camelCase that want to be in snake_case.
    • New functions without type hints.
    • New functions or classes without informative docstrings.
    • Changes to semantics not reflected in the relevant docstrings.
    • New or changed command line options for Toil workflows that are not reflected in docs/running/{cliOptions,cwl,wdl}.rst
    • New features without tests.
  • Comment on the lines of code where problems exist with a review comment. You can shift-click the line numbers in the diff to select multiple lines.
  • Finish the review with an overall description of your opinion.

Merger Checklist

  • Make sure the PR passes tests.
  • Make sure the PR has been reviewed since its last modification. If not, review it.
  • Merge with the Github "Squash and merge" feature.
    • If there are multiple authors' commits, add Co-authored-by to give credit to all contributing authors.
  • Copy its recommended changelog entry to the Draft Changelog.
  • Append the issue number in parentheses to the changelog entry.

mr-c
mr-c reviewed May 9, 2024
View reviewed changes
README.rst
programming. It supports running workflows written in either Common Workflow Language (`CWL`_) or
Workflow Description Language (`WDL`_), as well as having its own rich Python API for writing workflows against.
programming. It supports running workflows written in either Common Workflow Language (`CWL`_) 1.0-1.2 or
Workflow Description Language (`WDL`_) 1.0-1.1, as well as having its own rich Python API for writing workflows against.
Copy link
Contributor

Choose a reason for hiding this comment

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

WDL or OpenWDL? They are different things..

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that "OpenWDL" is an organization, and that "WDL" is a language, with "WDL 1.0" and "WDL 1.1" being language specifications published by OpenWDL.

Is there another "WDL 1.0" that isn't the same as the one OpenWDL wrote a spec for?

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that WDL draft 2, WDL 1.0, and "WDL development" is what Cromwell implements. OpenWDL 1.0 is different from (Cromwell) WDL 1.0. Cromwell doesn't support OpenWDL 1.1.

Copy link
Member Author

Choose a reason for hiding this comment

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

On Cromwell's page about language support they link to OpenWDL's spec for Draft 2. Or at least where it would be if it was still where they link to.

So I don't think Cromwell's WDL version number series is genuinely intended to be a different language or set of language versions.

Now, "WDL 1.0 as implemented by Cromwell" is not 100% identical to "WDL 1.0 as specified by OpenWDL". But I don't think that that means Cromwell thinks they have implemented the "real" WDL 1.0, it just means the spec and one of the popular implementations don't 100% match. And I haven't seen anyone on the OpenWDL side calling the language OpenWDL.

DailyDreaming
DailyDreaming approved these changes May 21, 2024
View reviewed changes
Copy link
Member

@DailyDreaming DailyDreaming left a comment

Choose a reason for hiding this comment

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

I think that having the versions in there vs. not having them is an improvement. If the terminology of WDL vs. OpenWDL is ambiguous, we can always change it in the future, but I think that this makes sense to me now.

@DailyDreaming DailyDreaming merged commit e8d85a4 into master May 21, 2024
1 check passed
@DailyDreaming DailyDreaming deleted the issues/4896-add-workflow-versions branch May 21, 2024 22:07
stxue1 pushed a commit that referenced this pull request May 23, 2024
@adamnovak @github-actions @stxue1
Add supported workflow language versions to README (#4923)
18d3c53 
This will fix #4896.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
stxue1 pushed a commit that referenced this pull request May 23, 2024
@adamnovak @github-actions @stxue1
Add supported workflow language versions to README (#4923)
23b9465 
This will fix #4896.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
stxue1 pushed a commit that referenced this pull request May 23, 2024
@adamnovak @github-actions @stxue1
Add supported workflow language versions to README (#4923)
033e541 
This will fix #4896.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mr-c mr-c mr-c left review comments

@DailyDreaming DailyDreaming DailyDreaming approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add supported language versions to README
3 participants
@adamnovak @mr-c @DailyDreaming