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

Existing branch handling #95

Closed
Tracked by #46
dominykas opened this issue Apr 18, 2021 · 3 comments · Fixed by #101
Closed
Tracked by #46

Existing branch handling #95

dominykas opened this issue Apr 18, 2021 · 3 comments · Fixed by #101
Labels

Comments

@dominykas
Copy link
Member

dominykas commented Apr 18, 2021

Scenarios:

  1. Suppose you're on a branch some-branch on the parent repo
  2. You run wiby test
  3. One of the dependent repos already has a branch called wiby-some-branch
    a) It could be that you already ran wiby test and did not wiby clean afterwards (most common)
    b) It could be that whoever was testing, wanted to make additional changes on the branch to get the tests to pass with the linked parent
    c) It could be that it exists, because multiple dependencies are being tested

Thinking out loud:

  • In cases (b) and (c) we probably need to link the parent into package.json and push a commit into an existing branch; if the parent is already linked, then we probably want to push an empty commit (git commit -m "This is a blank commit" --allow-empty) to re-trigger the builds.
  • In case (a), we probably want to delete the branch, recreate it from HEAD and link the parent again.
    • The problem here is that we don't know if we're dealing with (a) or (b)/(c) - in case of (b)/(c) we want to keep all changes
    • The simple approach would be to not rebase, however that may mean that wiby test is running against outdated code (i.e. it no longer answers the question "will I break you"...)
    • Maybe the right answer is to always attempt to update the existing branch with master, before pushing a new commit? Can that be done via the API? We could then notify the user about the conflicts, if any, via wiby results?

Related issues:

@ghinks
Copy link
Contributor

ghinks commented Apr 22, 2021

Thank you for thinking of these cases. Initially due to looking at this at the end of the day I did not understand the issues. But I think you have explained them. My turn to think aloud.

Essentially the wiby-some-branch may exist and there are different reasons to why it could be preexist. But these come down to 2 use cases ( a or b/c).

As a thought experiment I tried to rationalize methods of determining what the two cases are or avoiding having this situation.

avoid this situation

If the branch names where unique to a particular run of wiby wiby-branch-name-unique-id then the use cases would be known and the means to address them would be understood. However that would mean keeping track of the unique branch names raised locally. So that would make things complicated at a stage we don't want them to be complicated further.

Place a well known comment in the PR ( if one exists ) before we attempt to clean it. If as in the raise draft PR we check to see if a comment from clean was added.

accept the situation

If we accept that we cannot know what use case caused the branch to exist we can ask the user. This would require the user to understand things and would mean that wiby would essentially have to scan the repos for these situations before asking the question of the user.

Hope my thinking out load helps with this conversation.

The actual options are limited and in some sense is a help to working out a potential solution.

@dominykas
Copy link
Member Author

OK, so after discussing this in PM WG meeting, here's my current thinking:

  • If the target branch exists - we should re-use that target branch
  • If the target branch already has the parent linked, we should push an empty commit to re-trigger any associated actions
  • If the target branch exists, the dependent needs to use PRs, but the PR does not exist - we should open a new PR after pushing the commit
  • If the target branch exists, and already has a PR open - we don't need to do anything (i.e. things should get automatically triggered due to a new commit)

Going forward:

  • If the target branch is not up to date with the default branch, we should let the user know when they request a wiby result (new feature)
    • The above applies to wiby-[branch name] when it is not up to date with its corresponding [branch name], if one exists - not just the default branches
  • We could also build a new command wiby rebase (naming TBD - we probably don't want to do an actual git rebase), which would make sure that the target branches are up to date with their corresponding default branches wherever possible

@dominykas
Copy link
Member Author

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants