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 review fixes.skip links to source #4236

Merged
merged 5 commits into from
Jun 22, 2021

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented May 31, 2021

(dependent on #4233 )
These changes close #4170 and a minor issue without a ticket.

  • review: hide _cylc-install directory #4170 Prevent Cylc Review from following links back to source directory for workflows installed using Cylc Install.
  • Prevent Cylc Review from Displaying the runN symlink to the latest run folder.

[marked against a Cylc 8 milestone because this change only adds support for Cylc 8 workflows, and should not change an aspect of Cylc 7 functioning]

  • 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).
  • Does not need tests: legacy product, manual testing only.
  • No change log entry included & No documentation update required: Removing things that we don't want users to see before Gamma release.

To test

Install a workflow with Cylc 8.
Switch to Cylc 7 and start Cylc Review (cylc review start).
Ensure that you cannot see links to ~/cylc-run/<workflow>/_cylc-install or _cylc_install.
Ensure that you cannot see links to ~/cylc-run/<workflow>/runN

@wxtim wxtim self-assigned this May 31, 2021
@wxtim wxtim linked an issue May 31, 2021 that may be closed by this pull request
@wxtim wxtim added this to the cylc-8.0.0 milestone May 31, 2021
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0.0, cylc-7.8.x Jun 2, 2021
@oliver-sanders
Copy link
Member

Bumped to 7.8.x (otherwise it will show up as closed against the wrong codebase).

@oliver-sanders
Copy link
Member

Travis tests still working 👍.

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.

Tested as working.

In the process I noticed that a running Cylc 8 workflow was marked at "stopped", not sure if that's been spotted.

@wxtim wxtim requested review from datamel and removed request for dpmatthews and MetRonnie June 11, 2021 14:40
@oliver-sanders oliver-sanders requested review from datamel and removed request for datamel June 21, 2021 14:07
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 have checked out this branch and read the code. Tested as working. One minor typo spotted that pre-dates this pr. Not important enough to withhold merge though.

lib/cylc/review_dao.py Outdated Show resolved Hide resolved
Co-authored-by: Melanie Hall <37735232+datamel@users.noreply.github.com>
@wxtim wxtim merged commit a644163 into cylc:7.8.x Jun 22, 2021
@wxtim wxtim deleted the cylc-review-fixes.skip_links_to_source branch June 22, 2021 06:23
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.

review: hide _cylc-install directory
4 participants