Minor grammar fix on Pilot.JobCleanerQueuesExcluded - #1046
Merged
Conversation
bgentry
approved these changes
Sep 28, 2025
Comment on lines
+22
to
+24
| // JobCleanerQueuesExcluded returns queues that should be excluded from the | ||
| // main River client's JobCleaner. Empty omitted queues should return nil as | ||
| // instead of empty array. | ||
| // main River client's JobCleaner. If no queues should be omitted, this | ||
| // function should return nil as opposed to an empty array. |
Contributor
There was a problem hiding this comment.
What's the difference in behavior if I do return an empty array? Maybe worth spelling out if anything but I would expect any logic depending on this to consider them equivalent?
Contributor
Author
There was a problem hiding this comment.
K I added a little more text to that.
The reason is that we check on empty in the underlying database query and unlike Go, a null array and a zero-length array or not equivalent in Postgres. We could check on both those conditions, but I figured it'd be better to constrain our expectations so we don't pollute the SQL that much.
There was an extra word before ("as") that made this doc's grammar
wrong, but it was also just written in such a way that it was difficult
to parse. Here, do a bit of a rewrite as a fix up.
brandur
force-pushed
the
brandur-minor-grammar-fix
branch
from
September 28, 2025 16:58
ae5f376 to
b578da3
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
2 participants
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.
There was an extra word before ("as") that made this doc's grammar
wrong, but it was also just written in such a way that it was difficult
to parse. Here, do a bit of a rewrite as a fix up.