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

don't display bad practice #746

Merged

Conversation

wxtim
Copy link
Member

@wxtim wxtim commented Jun 20, 2024

Friend of metomi/rose#2783

We shouldn't do anything to encourage using directives equivalent to execution time limit.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.

@wxtim wxtim self-assigned this Jun 20, 2024
@wxtim wxtim requested a review from MetRonnie June 20, 2024 12:25
@wxtim wxtim added the small label Jun 20, 2024
@wxtim wxtim added this to the 8.3.x milestone Jun 20, 2024
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.

Friend of metomi/rose#2783

I assume you mean cylc/cylc-flow#6137

@MetRonnie
Copy link
Member

Actually, could be worth explicitly mentioning using execution timeout instead of directives?

@wxtim
Copy link
Member Author

wxtim commented Jun 21, 2024

@MetRonnie - Added a warning

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.

Rearrange execution time limit admonition
@MetRonnie MetRonnie merged commit 77396df into cylc:master Jun 21, 2024
1 check passed
@wxtim wxtim deleted the fix.do_not_display_using_wallclock_directives branch June 24, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants