Fix counting of released RCs #19
Open
+68
−2
Conversation
This file contains 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
Add more elaborate description to `--includeRC`. Related: - woocommerce/grow#65 - woocommerce/grow#66
eason9487
reviewed
Jul 24, 2023
There was a problem hiding this comment.
Thanks for the fix.
I guess it should be fine to proceed with merging as it works, but the code snippet looks a bit confusing. Wonder if it could apply the same change as woocommerce/grow#66?
const releasesAdded = output.filter(
release => (includeRC || !isRC(release)),
)| @@ -90,7 +92,7 @@ Fetching L-4 versions woocommerce! | |||
|
|
|||
| for (const version of versions) { | |||
| const releasesAdded = output.filter( | |||
| release => !isRC(release), | |||
| release => (includeRC || !isRC(release)), | |||
There was a problem hiding this comment.
Curious why the applied fix is not the same as it is in woocommerce/grow#66?
Comment on lines
+69
to
+70
| // Actually there is no quarantee, that the current L-x version will include any patches. | ||
| // But with a high offset, we increase the likelyhood of that. |
There was a problem hiding this comment.
"quarantee" and "likelyhood" look like typos.
Comment on lines
+102
to
+104
| // Actually there is no quarantee, that the current L-x version will include any RC. | ||
| // For example, if there wre x+1 patches, there will be no RC. | ||
| // But with a high offset, we increase the likelyhood of that. |
There was a problem hiding this comment.
Same typos as above? And the "wre" seems to be a typo as well.
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.
Changes proposed in this Pull Request:
--includeRC.Related:
Screenshots:
Detailed test instructions:
checkout and
npm run builduse
l-xcommand with--includeRC --includePatches, with an offset big enough, to get some RCs. LikeCheck that the number of results is equal to offset+1
Additional details: