Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pre-create waterfall branches in bump version action #113

Merged

Conversation

lavigne958
Copy link
Contributor

By creating the waterfall branches in bump version we ensure when opening the PR that bert-e is already happy and we don't need to solve conflict ourselves knowing that we don't want to keep the upade as the update only contains the version bump for version x.y and we try to merge in upper version branches wo th version is already x+1.y+1 and we don't want to forward-port this change.

By creating the waterfall branches in bump version we ensure
when opening the PR that bert-e is already happy and we don't need
to solve conflict ourselves knowing that we don't want to keep the upade
as the update only contains the version bump for version x.y
and we try to merge in upper version branches wo th version is already
x+1.y+1 and we don't want to forward-port this change.
@lavigne958 lavigne958 requested a review from a team September 11, 2023 08:55
@lavigne958 lavigne958 requested a review from a team as a code owner September 11, 2023 08:55
@lavigne958 lavigne958 self-assigned this Sep 11, 2023
@lavigne958
Copy link
Contributor Author

BTW: this action is only used by xcore projects for now.
If it works we plan to use the same script in artesca, but we won't be using this action we'll be using the action as it's for now custom to xcore only. we'll be just copying the script most probably

@nootal
Copy link
Contributor

nootal commented Sep 11, 2023

@lavigne958 interesting; I understand your PR as:

  • we don't leave to bert-e the responsibility to create integration branches, as it would create them the "dumb" way aka merging with upper branches, that would create conflicts in this case, and we would have to revert/ignore the changes
  • instead, we pre-create the integration branches with a script that will automatically resolve the conflicts with a specific strategy so we don't merge forward the bump and we keep what already exists in the upper branches
  • when the PR is eventually created, bert-e is happy from the start as all branches are here and it can directly use them
    Am I correct with this understanding?

If yes, I'm OK with you moving forward with this approach as it helps you in the short term, but we could do better for the greater good: we could add an option to bert-e to solve conflicts with a specific strategy, as you did; would look like a command /solve_conflicts_strategy_ours then bert-e would work as usual and solve them the way you want.
This would avoid your script replicating the internals of bert-e.

@lavigne958
Copy link
Contributor Author

Hi yes this is exactly what it is 🤩

hoooo my god this is so cool ! that would help us so much on each artesca release you have no idea.

So far as I said this is only used in xcore internal repo so that's safe to use I think.

but yes the bert-e approach would be awesome ! on behalf of all personal workinf on artesca CI: thank you so much

Copy link
Contributor

@tcarmet tcarmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving a comment that is optional to address, therefore approving.

Alexandre Lavigne added 3 commits September 12, 2023 13:54
In case of left over from previous failed run, cleanup any branches that could prevent
the workflow from running
@lavigne958 lavigne958 merged commit 9e042a6 into main Sep 12, 2023
3 checks passed
@lavigne958 lavigne958 deleted the feature/pre_create_waterfall_Branches_in_bump_version branch September 12, 2023 15:14
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