Always push tags with the github api#672
Conversation
🦋 Changeset detectedLatest commit: 3b4d426 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| sha: context.sha, | ||
| }); | ||
| } catch (err) { | ||
| // Assuming tag was manually pushed in custom publish script |
There was a problem hiding this comment.
Hm, it's pretty crucial for those tags to be created so we can create github releases properly. I know this is preexisting - but it would be great if we could only swallow specific errors here.
| // Assuming tag was manually pushed in custom publish script | ||
| core.warning(`Failed to create tag ${tag}: ${(err as Error).message}`); | ||
| } | ||
| await exec("git", ["push", "origin", tag], { |
There was a problem hiding this comment.
Hm, how sure are we this new flow will resolve the issue. The previous code wasn't exactly pushing all tags at once either, right?
There was a problem hiding this comment.
Hmm actually you might be right. I'm checking the timeline and we were pushing all git tags at once up until #391 which refactors to call them separately in parallel. After that's merged, we haven't got comments of it not working, so I guess it's fixed?
I was assuming since we push in parallel, GitHub might still detect it, and the REST API is able to workaround it. But maybe this isn't a problem now. I haven't personally tested this manually.
| @@ -114,25 +114,16 @@ export class GitHub { | |||
| } | |||
|
|
|||
| async pushTag(tag: string) { | |||
There was a problem hiding this comment.
We should add a comment here explaining why we are not using the git CLI at all here
|
Looks like this was already fixed so I'm going to close this. But the suggested improvements can be done later. |
2 participants
fix #669