rhys-vdw / ts-auto-guard

Generate type guard functions from TypeScript interfaces
MIT License
497 stars 54 forks source link

Merging two PRs simulataneously causes errors in CD #108

Closed rhys-vdw closed 2 years ago

rhys-vdw commented 3 years ago

Merging two PRs at once causes issues:

To https://github.com/rhys-vdw/ts-auto-guard
 ! [rejected]        master -> master (fetch first)
error: failed to push some refs to 'https://github.com/rhys-vdw/ts-auto-guard'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Error: Process completed with exit code 1.

https://github.com/rhys-vdw/ts-auto-guard/runs/1317630220?check_suite_focus=true

npm ERR! code E403
npm ERR! 403 403 Forbidden - PUT https://registry.npmjs.org/ts-auto-guard - You cannot publish over the previously published versions: 1.0.0-alpha.4.
npm ERR! 403 In most cases, you or one of your dependencies are requesting
npm ERR! 403 a package version that is forbidden by your security policy.

https://github.com/rhys-vdw/ts-auto-guard/runs/1317632530?check_suite_focus=true

cc @0xorial

Can we manually hit a "Release" button somewhere to try it again?

rhys-vdw commented 3 years ago

Hm, also now we're in a bad state where we've release 1.0.0-alpha.4, but have no corresponding commit due to above error. We can fix this w/ a manual release, but I think CD is going to fail until we do something.

0xorial commented 3 years ago

oooh, so actions run in parallel... (in retrospective - why wouldn't they)

0xorial commented 3 years ago

yeah, I think the easiest fix for now is to bump version manually to 1.0.0-alpha.4 on master.

regarding fixing it in general...

at the very least I think it would be better to reverse the order of npm publish and git push. if we first git push and if fails, we at least won't end up in inconsistent state between git and npm as we have now. and if for some reason npm publish fails and we trigger it again we will have versions in git that won't exist in npm, which doesn't seem that bad

I'll make a PR for that

rhys-vdw commented 3 years ago

at the very least I think it would be better to reverse the order of npm publish and git push.

Yeah, nice.

rhys-vdw commented 3 years ago

Sorry @0xorial there was in issue with #110: https://github.com/rhys-vdw/ts-auto-guard/actions/runs/336826302

The workflow is not valid. .github/workflows/cd.yml (Line: 12, Col: 9): Unexpected symbol: '='. Located at position 21 within expression: github.event_name === 'workflow_dispatch' || !startsWith(github.event.head_commit.message, '[ci skip]')

I wonder if there's a way to test these...

rhys-vdw commented 3 years ago

Hey @0xorial I think the easiest fix here would just to be make bump + release manual buttons instead of automatic on each merge. In the case where we want to merge a few changes and release it's going to make a bunch more sense. If we do it this way it's also okay to assume there will only be one release running at a time if easier, because we just won't do that.

Looks like it's possible: https://levelup.gitconnected.com/how-to-manually-trigger-a-github-actions-workflow-4712542f1960

I'm also open to moving the CI test over to GitHub too.

0xorial commented 3 years ago

@rhys-vdw, not sure if it is possible to test in an automated manner, especially since we are actually doing some external changes... but I guess we can do smth like

if (branch == 'master) {
  git push
  npm publish
} else {
  npm publish --dry-run
}

this would allow us to make a dry run on another branch to catch some issues

rhys-vdw commented 2 years ago

Fixed by manual publish trigger