Skip to content

Conversation

oliver-sanders
Copy link
Member

  • Add new field.
  • Abstract back-compat logic to make it easier to add and maintain support for older scheduler versions.

Nice-to-have for 8.6 if poss, but can be bumped.

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).
  • Changelog entry included if this is a change that can affect users - very minor
  • 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.

@oliver-sanders oliver-sanders added this to the 8.7.0 milestone Sep 23, 2025
@oliver-sanders oliver-sanders self-assigned this Sep 23, 2025
@hjoliver
Copy link
Member

Tests failing - just screenshots I think.

Happy to quickly review and test this tomorrow - it would make sense to get it in 8.6 to match the GUI change (although obvs not critical).

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Sep 24, 2025

Yes, non-critical.

Forgot to commit the new screenshot 🤦.

@oliver-sanders oliver-sanders force-pushed the tui.add.estimated-finish-time-field branch from a77214b to 3ec67d0 Compare September 24, 2025 08:53
* Add new field.
* Abstract back-compat logic to make it easier to add and maintain
  support for older scheduler versions.
@oliver-sanders oliver-sanders force-pushed the tui.add.estimated-finish-time-field branch from 3ec67d0 to 18063bc Compare September 24, 2025 10:56
@oliver-sanders
Copy link
Member Author

(FYI: the added line is covered by tests, however, codecov cannot see this as the Tui updater is running in a separate process)

Copy link
Member

@wxtim wxtim left a comment

Choose a reason for hiding this comment

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

  • Read Code
  • Tried to break it.

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.

3 participants