Add make_latest property to the GH release POST request during publish
#1157
Conversation
There was a problem hiding this comment.
thanks for restarting the effort and conversation around this one, @pfarnach. i think this does get us most of the way there, but we'll need to find a more dynamic approach that considers cases where the stable release branch is not named main
| @@ -0,0 +1,3 @@ | |||
| export default function isLatestRelease({ type, main }) { | |||
| return type === "release" && main ? "true" : "false"; | |||
There was a problem hiding this comment.
it wouldnt be safe to specifically target the main branch since master is still common and there are other patterns that teams use for naming their stable release branch. this would need to consider how the release branches are defined for the specific project
There was a problem hiding this comment.
Hi @travi thanks for looking at my PR!
I'm having trouble finding explicit docs about the branch object (the closest thing I could find is here) so I'm going off my very cursory understanding of the source code. It seems like main in this context is a boolean (rather than a string representing the branch name) that's set here, based on the index of the branch that's passed into the config. In any case, that also doesn't seem very reliable unless there's something ensuring the main/master/whatever branch is the first element there.
Would it be enough to just check the type value and remove the main check? From what I can tell, the other possible values are maintenance and prerelease but could use a gut-check on that.
There was a problem hiding this comment.
i think you did find the closest thing to documentation on that detail. with the lack of a better doc, i went spelunking through what the core package passes to this lifecycle method to refresh my understanding. looks like i landed in the same places you explored. some of this is from before i joined the project and doesnt always stick in my brain properly. i think you landed on the correct approach.
Would it be enough to just check the type value and remove the main check? From what I can tell, the other possible values are maintenance and prerelease but could use a gut-check on that.
main accounts for "next"-type releases. not a prerelease, not a maintenance release. normal release, but not to the default channel. so given that, main is needed for determining "latest" and you've handled it appropriately here.
sorry for my confusion before and thanks for your patience
| @@ -0,0 +1,3 @@ | |||
| export default function isLatestRelease({ type, main }) { | |||
| return type === "release" && main ? "true" : "false"; | |||
There was a problem hiding this comment.
i think you did find the closest thing to documentation on that detail. with the lack of a better doc, i went spelunking through what the core package passes to this lifecycle method to refresh my understanding. looks like i landed in the same places you explored. some of this is from before i joined the project and doesnt always stick in my brain properly. i think you landed on the correct approach.
Would it be enough to just check the type value and remove the main check? From what I can tell, the other possible values are maintenance and prerelease but could use a gut-check on that.
main accounts for "next"-type releases. not a prerelease, not a maintenance release. normal release, but not to the default channel. so given that, main is needed for determining "latest" and you've handled it appropriately here.
sorry for my confusion before and thanks for your patience
|
it looks like we have some failures in the integration tests. could you look into those? likely some small updates are needed based on these changes. also, it might be worth having an integration test or two for this scenario, so while youre fixing these up, please read through the existing tests to decide if that could be valuable. |
|
Appreciate the review, @travi -- I fixed the integration tests and added three more to cover the publishing-without-assets flow |
|
🎉 This PR is included in version 12.0.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
This PR implements the requested change from this stalled PR #884
And fixes #817