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

Add new job var CYLC_WORKFLOW_NAME_BASE. #5009

Merged
merged 4 commits into from
Jul 25, 2022

Conversation

hjoliver
Copy link
Member

@hjoliver hjoliver commented Jul 24, 2022

These changes close #4977

As requested: did not add CYLC_WORKFLOW_NAME_DIR

Small compromise: I did not remove CYLC_WORKFLOW_NAME because taken together it's immediately clear what the new variable is the basename of.

So now we have:

  • CYLC_WORKFLOW_ID - e.g. a/b/c/run1
  • CYLC_WORKFLOW_NAME - e.g. a/b/c
  • CYLC_WORKFLOW_NAME_BASE - e.g. c

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg and conda-environment.yml.
  • Appropriate tests are included (unit and/or functional).
  • Appropriate change log entry included.
  • No documentation update required.

@hjoliver hjoliver added this to the cylc-8.0.0 milestone Jul 24, 2022
@hjoliver hjoliver self-assigned this Jul 24, 2022
@hjoliver hjoliver marked this pull request as ready for review July 24, 2022 23:58
@wxtim wxtim self-requested a review July 25, 2022 07:55
@wxtim
Copy link
Member

wxtim commented Jul 25, 2022

I've assigned myself to do a code review so that @dpmatthews doesn't have to, and so that @oliver-sanders only has to do a second review.

tests/unit/test_job_file.py Outdated Show resolved Hide resolved
Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

  • Read
  • Checked out locally, and successfully run the changed tests.
  • 1 broken test - suggested fix in comment.
  • Thank you for getting rid of all those ${CYLC_WORKFLOW_NAMES}.

@wxtim wxtim self-requested a review July 25, 2022 08:37
Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

See comments.

Co-authored-by: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
@oliver-sanders
Copy link
Member

Opened docs PR - cylc/cylc-doc#506

@hjoliver
Copy link
Member Author

Docs PR merged. One review OK for this.

@hjoliver hjoliver merged commit 5658163 into cylc:master Jul 25, 2022
@hjoliver hjoliver deleted the workflow-name-base branch July 25, 2022 23:19
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.

CYLC_WORKFLOW_NAME: use the "flat" name
3 participants