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

Bugfix of job handling #18

Merged
merged 3 commits into from
Oct 11, 2021
Merged

Bugfix of job handling #18

merged 3 commits into from
Oct 11, 2021

Conversation

hlts2
Copy link
Contributor

@hlts2 hlts2 commented Oct 9, 2021

This pr contains the following implementations.

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
}
}
if len(st.errJobs) != 0 {
return nil, errors.New(st.Detail())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for the following issue.
#14

rytswd
rytswd previously approved these changes Oct 9, 2021
Copy link
Contributor

@rytswd rytswd left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM - just a quick one though, this won't fix the issue we saw internally with some job being picked up incorrectly for the total number of jobs?

@rytswd
Copy link
Contributor

rytswd commented Oct 9, 2021

Also, you seem to have not included any fix for the verbose error message when the job fails?

@hlts2
Copy link
Contributor Author

hlts2 commented Oct 11, 2021

@rytswd

Thanks, LGTM - just a quick one though, this won't fix the issue we saw internally with some job being picked up incorrectly for the total number of jobs?

I think the implementation of this link has solved that problem.

Also, you seem to have not included any fix for the verbose error message when the job fails?

if the job fails, we can get failed job name.ref:
https://github.com/upsidr/merge-gatekeeper/blob/bugfix/fix-timing-to-finish-job/internal/validators/status/status.go#L20-L21. but I could not delete CLI usage information. 🙏
So I will try this with another PR.

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>
@hlts2 hlts2 merged commit 4e654cb into main Oct 11, 2021
@hlts2 hlts2 deleted the bugfix/fix-timing-to-finish-job branch October 11, 2021 11:21
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.

2 participants