Skip to content
This repository has been archived by the owner on Oct 25, 2022. It is now read-only.

Git plugin: create branch if it does not exist #247

Merged
merged 2 commits into from
Jun 1, 2021

Conversation

easybe
Copy link
Contributor

@easybe easybe commented Jun 1, 2021

Note that if a local branch with the same name already exist, it will be reset to the current HEAD.

@easybe easybe force-pushed the git branch 2 times, most recently from 3b83454 to e59a1d5 Compare June 1, 2021 06:59
@majkinetor
Copy link
Owner

Why would we need flag for this ? Can't you just create a branch if it doesn't exist by default ?

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

Yes, I guess that could work for me.

However,

git pull -q origin $Branch

is also a problem for me. IMHO a git pull does not really make sense here, as we have uncommitted changes at this point which might lead to an error. Or am I missing something here?

Is it an option to change the current behavior?

@majkinetor
Copy link
Owner

That is for collaborative environment where somebody else might push to that branch in the middle of the CI pipeline. It ensures there are no conflicts on push.

@majkinetor
Copy link
Owner

majkinetor commented Jun 1, 2021

That is also not a problem, you could ignore error on it or (better) do it for existing branches only.

@easybe easybe changed the title Git plugin: add option to create new branch Git plugin: create branch if it does not exist Jun 1, 2021
@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

OK, I updated the PR.

Note that if a local branch with the same name already exist, it will be reset to the current HEAD.

I hope that is OK.

@easybe easybe force-pushed the git branch 2 times, most recently from 4a981ae to 7b87e26 Compare June 1, 2021 15:27
@majkinetor
Copy link
Owner

You miss -q everywhere. Without it CI may fail.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

You miss -q everywhere. Without it CI may fail.

I would prefer it that way, but OK, I can add it.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

Done

@majkinetor
Copy link
Owner

I would prefer it too, but CI fails because git writes to stderr if q is not present.

Note that if a local branch with the same name already exist, it will be
reset to the current HEAD.
@majkinetor
Copy link
Owner

majkinetor commented Jun 1, 2021

So with this change, you can basically make persistence commits visible in separate branch and not pollute your master branch with bot commits ? Its enough for me to set some arbitrary non existent branch like -Branch updatebot ?

I remember some people asked for it a long time ago.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

Yes, exactly. I create PRs from these branches.

@majkinetor
Copy link
Owner

majkinetor commented Jun 1, 2021

So, are you done and checked if it works in real AU update ? This plugin is used by everybody.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

Yes, I am done.

It works in my set up which is based on https://github.com/majkinetor/au-packages-template/blob/master/update_all.ps1.

Not sure how to test it more.

@majkinetor majkinetor merged commit 0493e66 into majkinetor:master Jun 1, 2021
@majkinetor
Copy link
Owner

majkinetor commented Jun 1, 2021

Ah, wait... It won't work like this I think. If packages are on master and you are pulling updatebot you will get nothing. So you need to get master and switch to another branch after that if I am not mistaken. So there need to be 2 branches as parameters - SourceBranch and DestinationBranch

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

For me it works because:

  • CI checks out the latest master
  • AU creates a new branch from master
  • AU runs git pull which fails
  • AU pushes to the new branch
  • I can merge branch into master if I want

For my case "pull" does not make sense...

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

If you specify master as the branch, nothing should have changed.

@majkinetor
Copy link
Owner

It was there as a safety because time passes since CI pulls the master and AU finishes and your changes are "saved". If anybody commits during that time you can't push on the same branch any more. Contrary to that, pull immediately before commit makes that highly unlikely.

Now it works for you because of your specific setup. It may not work for people who use the same branch for source and destination and have more active users.

So,

  1. CI checks out the latest master
  2. CI script runs init
  3. CI scripts runs AU
  4. AU runs for some time
  5. Somebody commits on master <========= THIS
  6. AU finishes
  7. AU tries to push to master and fails.

@majkinetor
Copy link
Owner

In case Branch is different then master, then I agree, pull is probably not needed UNLESS again somebody commits to it, like you have multiple AU scripts for some reason operating for redundancy.

@majkinetor
Copy link
Owner

Other problem is that you don't need to have CI - you can perfectly have Git plugin working without any CI whatsoever, then it needs to pull master if AU runs from 2 locations .

@majkinetor
Copy link
Owner

So maybe to be sure, we should have 2 params, SourceBranch aka Branch as before (param alias) and DestinationBranch. If you use CI and you are lone developer you could probably just use DestinationBranch and CI will do the rest.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

I really don't see any problem here. As I said,

If you specify master as the branch, nothing should have changed.

because a git pull is still performed.

If you use a different branch, worst case would be that you have a merge conflict when merging the branch. Then you could e.g. re-run or rebase.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

If you have two AU scripts pushing to the same branch, the slower one will pull the newly created branch.

@easybe
Copy link
Contributor Author

easybe commented Jun 1, 2021

I like this solution because the changes are very minimal 🤷‍♂️

@majkinetor
Copy link
Owner

It works on my repo, branch name updates

Not sure why it triggered new build ASAP after bot commit on AppVeyor ... need to look into it.

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

Successfully merging this pull request may close these issues.

2 participants