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

No recursive installation #4442

Merged
merged 33 commits into from
Nov 22, 2021
Merged

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Sep 30, 2021

These changes close #4396

  • 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.py and
    conda-environment.yml.
  • Appropriate tests are included:
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at No recursive installation cylc-doc#302

@wxtim wxtim added this to the cylc-8.0rc1 milestone Sep 30, 2021
@wxtim wxtim self-assigned this Sep 30, 2021
@wxtim wxtim marked this pull request as draft September 30, 2021 10:35
@wxtim wxtim requested review from MetRonnie and removed request for MetRonnie September 30, 2021 13:14
@wxtim wxtim marked this pull request as ready for review October 1, 2021 07:01
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
@wxtim wxtim marked this pull request as draft October 5, 2021 10:04
@wxtim
Copy link
Member Author

wxtim commented Oct 5, 2021

Converted back into a draft pending a re-examination after reviewing #4448

@wxtim
Copy link
Member Author

wxtim commented Oct 7, 2021

@MetRonnie I've done a fair re-write... Sorry.

@wxtim wxtim marked this pull request as ready for review October 7, 2021 13:07
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
cylc/flow/workflow_files.py Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie November 15, 2021 12:10
@wxtim wxtim marked this pull request as ready for review November 15, 2021 15:46
@wxtim wxtim requested a review from MetRonnie November 16, 2021 15:24
@wxtim
Copy link
Member Author

wxtim commented Nov 16, 2021

@datamel - much water has passed under the bridge since your last review. Might I ask you to have another look.

@wxtim wxtim dismissed datamel’s stale review November 16, 2021 16:38

Massive change since approval

@MetRonnie
Copy link
Member

@datamel You might want to wait until after I've opened my PR against this PR, will contain a couple of fixes

Fix duplicate check for nested dirs
@MetRonnie
Copy link
Member

@datamel This is ready for review

CHANGES.md Outdated Show resolved Hide resolved
@wxtim wxtim requested a review from MetRonnie November 22, 2021 09:48
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.

No problems spotted. Code read, manually tested and run the tests locally.

Thanks @wxtim and @MetRonnie!

@datamel datamel merged commit 806af70 into cylc:master Nov 22, 2021
@wxtim wxtim deleted the no-recursive-installation branch November 22, 2021 16:11
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.

Disallow nested installation dirs
3 participants