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

Check run name as well as workflow name in Cylc install #6264

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Check run name as well as workflow name in Cylc install #6264

merged 1 commit into from
Aug 9, 2024

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jul 25, 2024 •
edited
Loading

Closes #6246

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 if present).
  • Tests are included (or explain why tests are not needed).
  • Changelog entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@wxtim wxtim self-assigned this Jul 25, 2024
@wxtim wxtim added bug Something is wrong :( small labels Jul 25, 2024
@wxtim wxtim added this to the 8.3.4 milestone Jul 25, 2024
@wxtim wxtim force-pushed the validate_full_workflow_ID branch 2 times, most recently from d92d7c6 to 587626d Compare July 25, 2024 13:17
MetRonnie
MetRonnie reviewed Jul 25, 2024
View reviewed changes
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Lint failing btw

changes.d/6264.fix.md Outdated Show resolved Hide resolved
cylc/flow/install.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
@wxtim wxtim force-pushed the validate_full_workflow_ID branch 2 times, most recently from 4859cde to 7a14867 Compare July 26, 2024 07:38
MetRonnie
MetRonnie requested changes Jul 26, 2024
View reviewed changes
cylc/flow/install.py Outdated Show resolved Hide resolved
@wxtim wxtim force-pushed the validate_full_workflow_ID branch from 7a14867 to 2c0d413 Compare July 26, 2024 10:13
@wxtim wxtim requested a review from MetRonnie July 26, 2024 10:14
MetRonnie
MetRonnie requested changes Jul 29, 2024
View reviewed changes
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

See comment above; requested a test

@wxtim wxtim requested a review from MetRonnie July 31, 2024 10:41
MetRonnie
MetRonnie requested changes Jul 31, 2024
View reviewed changes
tests/unit/test_install.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie July 31, 2024 12:19
MetRonnie
MetRonnie previously approved these changes Jul 31, 2024
View reviewed changes
@MetRonnie MetRonnie self-requested a review July 31, 2024 15:31
@MetRonnie MetRonnie dismissed their stale review July 31, 2024 15:31

Oliver noticed thing

@wxtim wxtim force-pushed the validate_full_workflow_ID branch from 0ce3561 to e040d9c Compare August 1, 2024 06:17
@wxtim wxtim requested a review from oliver-sanders August 1, 2024 06:17
MetRonnie

This comment was marked as resolved.

Sign in to view

@wxtim
Check run name as well as workflow name in Cylc install
7e84005 
Update changes.d/6264.fix.md

Co-authored-by: Ronnie Dutta <[email protected]>

Apply suggestions from code review

Co-authored-by: Ronnie Dutta <[email protected]>

Restore workflow_files

added tests for reserved dir names
@wxtim wxtim force-pushed the validate_full_workflow_ID branch from e040d9c to 7e84005 Compare August 1, 2024 10:52
@wxtim wxtim requested a review from MetRonnie August 1, 2024 10:52
@MetRonnie

This comment was marked as outdated.

Sign in to view
@MetRonnie MetRonnie changed the base branch from master to 8.3.x August 9, 2024 14:51
@MetRonnie
Copy link
Member

Kicking tests after changing base

@MetRonnie MetRonnie closed this Aug 9, 2024
@MetRonnie MetRonnie reopened this Aug 9, 2024
MetRonnie
MetRonnie approved these changes Aug 9, 2024
View reviewed changes
Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

Just flaky TUI test failing

@MetRonnie MetRonnie merged commit 00ba50d into cylc:8.3.x Aug 9, 2024
49 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MetRonnie MetRonnie MetRonnie approved these changes

@oliver-sanders oliver-sanders oliver-sanders approved these changes

Assignees

@wxtim wxtim

Labels
bug Something is wrong :( small
Projects
None yet
Milestone
8.3.4
Development

Successfully merging this pull request may close these issues.

validate full workflow ID
3 participants
@wxtim @MetRonnie @oliver-sanders