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

Lint: Add a check for indentation being 4N spaces. #5772

Merged
merged 9 commits into from
Nov 16, 2023

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Oct 17, 2023

Make 'url' key optional in check specification.
Document pyproject.toml fields.

A couple of small fixes I found when looking at Cylc Lint for my talk on Thurs.

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.

Make 'url' key optional in check specification.
Document pyproject.toml fields.
@wxtim wxtim self-assigned this Oct 17, 2023
@wxtim wxtim added small doc Documentation labels Oct 17, 2023
@wxtim wxtim added this to the cylc-8.2.3 milestone Oct 17, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.3, cylc-8.2.4 Nov 1, 2023
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

The multiple of four space check works well.

I spotted what looks like a duplicate code going through the CLi help:

U001:
    "[scheduling][dependencies][<recurrence>]graph =" -> "[scheduling][graph]<recurrence> ="

U002:
    "[scheduling][dependencies][<recurrence>]graph =" -> "[scheduling][graph]<recurrence> ="

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
@oliver-sanders oliver-sanders requested review from oliver-sanders and removed request for MetRonnie November 14, 2023 17:28
@wxtim
Copy link
Member Author

wxtim commented Nov 15, 2023

@oliver-sanders U001 and U002 being the same is deliberate, but I agree, perhaps a little confusing. They are detecting two different patterns with the same underlying cause (The change from [scheduling][dependencies][<reccurrance>]graph=, but one of them is detecting the graph= and one the [[dependencies]]).

I have added a bracket with a little text to this effect to each message - hopefully that makes it OK for you?

@markgrahamdawson markgrahamdawson dismissed their stale review November 16, 2023 11:46

Pull request may be revisited after discussion with Tim and Oliver

@markgrahamdawson markgrahamdawson merged commit 88b2798 into cylc:8.2.x Nov 16, 2023
19 of 23 checks passed
wxtim added a commit to wxtim/cylc that referenced this pull request Dec 5, 2023
…at_runtime

* upstream/master:
  log_vc_info: handle long command output (cylc#5821)
  GH Actions: limit tutorial workflow to Py3.11
  remove cylc task dependencies env var (cylc#5836)
  Refactor.lint (cylc#5718)
  protobuf 4.24.4 upgrade (cylc#5828)
  made reinstall work on multiple workflows
  Fix `IndepQueueManager` test (cylc#5832)
  Lint: Add a check for indentation being 4N spaces. (cylc#5772)
@wxtim wxtim deleted the lint.minor_fixes branch January 16, 2024 09:38
@wxtim wxtim mentioned this pull request Jan 23, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants