Closed ghost closed 6 months ago
@jessew-albert thank you for this suggestion! I am always pleased to learn about new ways folks are using this Action. I already have a few ideas in mind for how to implement a new feature that would enable your team to use this workflow better. The first idea that comes to mind would be implementing a new input option into this Action that allows you to configure what branch this Action is looking at to determine its "base".
I'm currently on holiday traveling around the UK and Scandinavia, but I have a few long train rides coming up so perhaps I can work on opening a PR while riding a railway 🚂 .
Thanks! ❤️
@jessew-albert Clarifying question for you:
Are you looking to have this Action internally compare a different branch than the pull request base? Currently, this Action only cares about the branch that your pull request is merging into. So if your stack of PRs is out-of-date with main
but not out-of-date with each other, then you should be okay.
To think about it visually, this Action will warn/force update the branch if the "update branch" (gray button), exists at the bottom of your pull request.
@GrantBirki not sure I'm following your question - the current behavior is what you described, i.e. if the stack of PRs is out of date with main
but not out of date with each other, then branch deploy will allow it to be deployed... but I'm looking for a way to get branch deploy to consider a branch out of date if it is out of date with main
.
In context of how my team uses the action, our main
branch maps 1:1 to our dev environment (i.e. when things get merged to main
, they auto-deploy to dev). Then branch-deploy can be used to briefly, sort of "temporarily" deploy changes on feature branches to the dev environment while testing them but before merging to main
. We don't want to have a branch get deployed to dev that is behind the main
branch though. That is, there should never be code deployed to dev that is missing commits that are on main
. That should hold true whether the base of the PR is main
or another feature branch (as is common when using stacked diff tooling such as Graphite).
Here's a diagram of when I'd expect branch-deploy to warn about an outdated branch:
gitGraph
commit id: "A"
commit id: "B"
branch featureA
checkout featureA
commit id: "featA1"
commit id: "featA2"
branch featureB
checkout featureB
commit id: "featB1"
commit id: "featB2"
checkout main
commit id: "C"
commit id: "D"
In this scenario, the gray "update branch" button would not show in featureB
's PR, because featureB
is not out of date with its base (featureA
).
However, if I tried to deploy featureB
to dev using branch-deploy, that shouldn't be allowed, because featureB
is missing the 2 most recent commits to main
- therefore, if I deployed featureB
to dev, I would effectively be rolling dev back 2 commits unexpectedly. In order for deploying featureB
to be allowed, we'd need to either merge main
into featureB
, i.e.
gitGraph
commit id: "A"
commit id: "B"
branch featureA
checkout featureA
commit id: "featA1"
commit id: "featA2"
branch featureB
checkout featureB
commit id: "featB1"
commit id: "featB2"
checkout main
commit id: "C"
commit id: "D"
checkout featureB
merge main
or rebase the stack with featureA
and featureB
on top main
, i.e.
gitGraph
commit id: "A"
commit id: "B"
commit id: "C"
commit id: "D"
branch featureA
checkout featureA
commit id: "featA1"
commit id: "featA2"
branch featureB
checkout featureB
commit id: "featB1"
commit id: "featB2"
In my opinion this feels like it should be the default behavior of branch deploy, although it's possible I'm wrong and that there's workflows out there that wouldn't want this behavior. I think if this behavior could be configured via an input that should help accommodate any workflow... please let me know if I'm making sense here!
Also thanks very much for your effort, I really love this action!
@jessew-albert thank you so much for your very detailed explanation! It was the diagrams that really helped me wrap my head around a solution here so I am very grateful for them (they even made it into the docs!).
I have published a pre-release version of this Action and have done a lot of acceptance testing with the new changes. The release is v9.0.0
.
Please go ahead and give the release a go and see if the new default outdated_mode: "strict"
works for you and solves this issue. If all looks okay, let me know and I'll close out this issue and publish the release officially.
Thank you for all the help here! 🎉
Happy deployments 🚀
Thanks so much for the work @GrantBirki! I just tried out the 9.0.0 release and it's all looking good to me!
My team relies on the branch-deploy functionality to prevent deploying branches that are out of date (i.e.
update_branch = "warn"
). We noticed recently that there's a nuance to this though: it only prevents deploying PRs that are out of date with their base branch.My team uses a stacked diffs workflow; the main relevant effect of that here is that it's not uncommon for branches to be stacked on top of each other - i.e.
featureA
's base branch ismain
, butfeatureB
's base branch isfeatureA
. As-is, branch deploy would allow us to deployfeatureB
even if the stack includingfeatureA
andfeatureB
is out of date when comparing againstmain
.For the time being we have a workaround by just adding our own condition to our action that checks the result of
but in an ideal world, it would be great if this could be implemented by branch-deploy internally.
Something along the lines of an input like
outdated_mode
with optionspr_base
anddefault_branch
(defaulting topr_base
to prevent this being a breaking change).