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

Full job id matching #740

Merged
merged 6 commits into from
Oct 4, 2024
Merged

Conversation

madwort
Copy link
Contributor

@madwort madwort commented Sep 25, 2024

@Providence-o Providence-o force-pushed the madwort-and-providence/full-job-id-matching branch from 424e8c0 to 416d649 Compare September 27, 2024 15:28
@madwort madwort force-pushed the madwort-and-providence/full-job-id-matching branch from f8547b0 to ef497e4 Compare October 3, 2024 15:18
@Providence-o Providence-o marked this pull request as ready for review October 3, 2024 15:53
Copy link
Contributor

@evansd evansd left a comment

Choose a reason for hiding this comment

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

This looks correct to me, and really nice to see the test coverage improving here as well. Great job!

I've pointed out a possible simplification, but it's not a big deal and I'm happy for this to go as is.

When you come to merge this, instead of merging directly could you click the down arrow and use the "Squash and merge" option?
image

We generally use the "Create a merge commit" option only when the commits have been specifically crafted to make sense on their own. And when the commits just reflect the development workflow we use "Squash and merge" which means they end of as a single commit in the project history.

jobrunner/cli/kill_job.py Outdated Show resolved Hide resolved
@Providence-o Providence-o enabled auto-merge (squash) October 4, 2024 16:25
@Providence-o Providence-o merged commit 4b0074f into main Oct 4, 2024
20 checks passed
@Providence-o Providence-o deleted the madwort-and-providence/full-job-id-matching branch October 4, 2024 16:26
@madwort
Copy link
Contributor Author

madwort commented Oct 9, 2024

Nb. my gpg key had expired whilst I was working on this, hence Unverified commits.

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.

kill_job cli command should not require confirmation when full job ID is specified
3 participants