Skip to content

feat: add branch input to configure the pull request source - #168

Open
dmregister wants to merge 2 commits into
changesets:mainfrom
OfferUp:feat/add-branch-input
Open

feat: add branch input to configure the pull request source#168
dmregister wants to merge 2 commits into
changesets:mainfrom
OfferUp:feat/add-branch-input

Conversation

@dmregister

Copy link
Copy Markdown
Contributor

No description provided.

@changeset-bot

changeset-bot Bot commented Apr 5, 2022

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4395c7e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@changesets/action Minor

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

@Andarist

Andarist commented Apr 6, 2022

Copy link
Copy Markdown
Member

Could you give some context behind this change? Why do you want to have this configurable?

@dmregister

Copy link
Copy Markdown
Contributor Author

@Andarist this was an issue with our CI system as it operates using webhooks. In some cases, actions were faster than webhooks and the build queue, in which this action would be triggered immediately to re-open the PR after it was merged and force push the same branch. This would alter the branch before our CI system checkout out the code and caused issues. The solution was to make the branch configurable and append the current version to branch e.g.
changeset-release/main-1.0.0. The version would be different next time the PR was re-opened (after all changesets are applied).

@Andarist

Andarist commented Apr 7, 2022

Copy link
Copy Markdown
Member

I understand that some race conditions might happen (with GitHub Actions we propose using concurrency config to deal with this) but the described situation is a little bit fuzzy to me. Could you list all the things that were happening during the race condition, step by step? This is especially a little bit confusing since I assume that you are already using GitHub Actions, so I wonder why concurrency doesn't solve your issue.

Also - how do you compute the "version suffix"?

@dmregister
dmregister force-pushed the feat/add-branch-input branch from b595c5e to e80eed2 Compare April 7, 2022 18:06
@dmregister

Copy link
Copy Markdown
Contributor Author

This input solves issues where not all pipelines are using github actions. These external pipelines perform actions when PR's are merged, so the first step of the external pipelines are to checkout the code that was just merged by this changeset PR. If there is any delay in the delivery of those webhooks that trigger the external pipelines, that is when the issue can occur. For instance, if you merge this changeset PR and there is a delay in webhook delivery, the changeset github action will immediately re-open the PR (because the act of merging the changeset PR is one of the events tracked) and force push to the same branch. Now when the external pipelines try to find the commit on the changeset branch, it's not the same and causes issues.

Also our external pipelines like to clean branches after they are merged, so this also allows them to delete once they have completed their task.

Computing the version is just using the package.json version to create the branch:
VERSION=$(jq -r '.version' package.json)

@dmregister

Copy link
Copy Markdown
Contributor Author

@Andarist another use-case is we use this in the mono-repo setup. We have 2 projects that want to use this functionality and without being able to configure the branch, it would not be possible.

@dmregister

Copy link
Copy Markdown
Contributor Author

@Andarist is this something you are interested in?

@supersabillon

Copy link
Copy Markdown

This PR would unblock our team from using changesets. Because the way our infrastructure is set up (all our branches are deployed to AWS and it uses the branch name in the domain), we have a requirement of not allowing forward slashes in the branch name. We would love to be able to configure this.

@Andarist

Copy link
Copy Markdown
Member

Part of the reason why I'm hesitant to add this is that it has a chance of breaking some things, such as this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants