refactor send_messages to remove an almost entirely duplicate code path#3297
Merged
refactor send_messages to remove an almost entirely duplicate code path#3297
Conversation
Contributor
Author
|
This doesnt really fix a bug so much as it improves the quality of the code (yes theres the detail that the original version of this introduced a bug but that was already reverted so for all intents and purposes this is a new refactoring change) |
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
7312fbe to
9ebc4eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This change is identical to that proposed in #3271, except it contains a fix for the problem where other jobs were not being scheduled and thus had to be reverted (in #3292)
In reviewing the code, there was a spot in there that was intended to exit early and skip that iteration. I had used
return(which exits the whole function) in the refactor rather thancontinue(which skips one loop iteration) because I was focused on deduplicating the if statement and didnt see that it was in a loop.This is a refactor and doesnt fix an explicit issue, but helps generally reduce tech debt
Notes for Reviewers
When testing, ensure that all jobs are getting scheduled correctly.
Signed commits