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

Feat.lint obsolete vars #5879

Merged
merged 5 commits into from
Dec 20, 2023
Merged

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Dec 14, 2023

Another item from Oliver's wishlist at #4917:

Search for deprecated items from this list

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).
  • 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.

@wxtim wxtim force-pushed the feat.lint-obsolete-vars branch 3 times, most recently from edf6c98 to 5cbb2cf Compare December 14, 2023 11:13
@wxtim wxtim self-assigned this Dec 14, 2023
@wxtim wxtim added this to the cylc-8.3.0 milestone Dec 14, 2023
@wxtim wxtim added the could be better Not exactly a bug, but not ideal. label Dec 14, 2023
changes.d/5879.feat.md Outdated Show resolved Hide resolved
Comment on lines 115 to 116
'task_url': 'URL from task metadata',
'workflow_url': 'workflow_URL from workflow metadata',
Copy link
Member

Choose a reason for hiding this comment

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

What doe these mean, "from task/workflow metadata"?

Copy link
Member Author

@wxtim wxtim Dec 14, 2023

Choose a reason for hiding this comment

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

You can set these items in

# flow.cylc
[meta]
    workflow_url = "www.hat.ever"

[runtime]
    [[task]]
        [[[meta]]]
            task = "some://url"

And they will be available. If you don't, they won't.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right. I'm not sure this is clear and it might be good to be able to link to an example in the docs. However the closest I could find is this section in https://cylc.github.io/cylc-doc/latest/html/user-guide/writing-workflows/runtime.html#task-event-template-variables

image

Unfortunately the specific item is not linkable, only the section heading quite far up

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree. I'll have a think about how this might work.

Copy link
Member Author

Choose a reason for hiding this comment

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

@MetRonnie - Have another look, I should think that it's better now.

cylc/flow/scripts/lint.py Outdated Show resolved Hide resolved
wxtim and others added 2 commits December 14, 2023 15:16
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
Co-authored-by: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com>
@wxtim wxtim requested a review from MetRonnie December 14, 2023 15:31
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.

Ah wait, got test failures

@wxtim wxtim marked this pull request as draft December 15, 2023 15:43
@wxtim wxtim marked this pull request as ready for review December 15, 2023 16:28
@wxtim wxtim requested a review from MetRonnie December 15, 2023 16:28
@wxtim
Copy link
Member Author

wxtim commented Dec 15, 2023

Ah wait, got test failures

Tests earning their keep. Hopefully Will pass now.

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.

👍

@wxtim wxtim mentioned this pull request Dec 19, 2023
14 tasks
@markgrahamdawson markgrahamdawson merged commit 031fc05 into cylc:master Dec 20, 2023
22 of 25 checks passed
@wxtim wxtim deleted the feat.lint-obsolete-vars branch December 20, 2023 13:13
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 9, 2024
* upstream/master: (350 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  set CYLC_DEBUG env var when set-verbosity DEBUG (cylc#5854)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Update copyright year
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  id: catch traceback in ID parsing
  actions: downgrade macos runners to macos 11 (cylc#5892)
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Add chained offset logic for FCP (cylc#5885)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  ...
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 11, 2024
* upstream/master: (81 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  Bump actions/upload-artifact from 3 to 4
  Bump actions/download-artifact from 3 to 4
  tests/i: update tui screenshot
  Update .github/workflows/test_fast.yml
  actions: add non-utc job to fast tests
  ...
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 12, 2024
* upstream/master: (81 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  Bump actions/upload-artifact from 3 to 4
  Bump actions/download-artifact from 3 to 4
  tests/i: update tui screenshot
  Update .github/workflows/test_fast.yml
  actions: add non-utc job to fast tests
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
could be better Not exactly a bug, but not ideal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants