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

Cylc lint fixes #5363

Merged
merged 7 commits into from
Feb 14, 2023
Merged

Cylc lint fixes #5363

merged 7 commits into from
Feb 14, 2023

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Feb 10, 2023

A ticket combining a number of small fixes for Cylc lint:

Closes:

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.
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md 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.

- Modified original Regex to test for _any_ letter being lowercase
- Added regex for FAMILY:trigger-all and trigger-any
@wxtim wxtim self-assigned this Feb 10, 2023
@wxtim wxtim added bug Something is wrong :( small labels Feb 13, 2023
@wxtim wxtim added this to the cylc-8.1.2 milestone Feb 13, 2023
@wxtim
Copy link
Member Author

wxtim commented Feb 13, 2023

Check tick boxes in #4917 on merge.

cylc/flow/scripts/lint.py Outdated Show resolved Hide resolved
cylc/flow/scripts/lint.py Outdated Show resolved Hide resolved
cylc/flow/scripts/lint.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie February 13, 2023 13:37
@wxtim
Copy link
Member Author

wxtim commented Feb 13, 2023

Thanks for spotting the issues with those regexes @MetRonnie

Copy link
Contributor

@datamel datamel left a comment

Choose a reason for hiding this comment

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

I've checked this out and manually tested the change. Fixes all the documented bugs.

Thanks @wxtim.

@MetRonnie
Copy link
Member

Thanks for spotting the issues with those regexes @MetRonnie

I'm not seeing any new commits. Still need to push changes?

Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@wxtim
Copy link
Member Author

wxtim commented Feb 14, 2023

Thanks for spotting the issues with those regexes @MetRonnie

I'm not seeing any new commits. Still need to push changes?

Nope - still need to avoid pushing changes which over-ride the comments I enacted using github. :(

@MetRonnie MetRonnie changed the base branch from master to 8.1.x February 14, 2023 10:32
@MetRonnie
Copy link
Member

This was open against master but on 8.1.2 milestone, I have updated the base branch to 8.1.x

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 codecov upload failing

@MetRonnie MetRonnie merged commit d501499 into cylc:8.1.x Feb 14, 2023
@wxtim wxtim deleted the cylc_lint_fixes branch February 14, 2023 11:08
wxtim added a commit to wxtim/cylc that referenced this pull request Feb 17, 2023
…lc into fix_localhost_platform_matching

* 'fix_localhost_platform_matching' of github.com:wxtim/cylc:
  Fix a bug preventing `cylc vip --workflow-name=foo` from working. (cylc#5349)
  fix no rose vars in cylc view (cylc#5367)
  Cylc lint fixes (cylc#5363)
  data store: support unsatisfied ext_trigger
  fix mypy fail caused by python/mypy#13969
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( small
Projects
None yet
3 participants