[Contrib] Checkout a PR

[sapk]

Could be impoved, but it seems to work.

For example, to check the PR #6007 simply do :
go run contrib/pr/checkout.go 6007
OR
make pr PR=6007

This will checkout PR and start with integration tests fixtures loaded.

There still seems to have problem with auth after login the user is not kept logged (and some repo are missing but that normal)

[codecov-io]

Codecov Report

Merging #6021 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #6021      +/-   ##
=========================================
- Coverage   38.82%   38.8%   -0.02%     
=========================================
  Files         355     355              
  Lines       50253   50253              
=========================================
- Hits        19510   19502       -8     
- Misses      27913   27922       +9     
+ Partials     2830    2829       -1

Impacted Files	Coverage Δ
models/unit.go	0% <0%> (-14.29%)	⬇️
models/repo_list.go	66.84% <0%> (-1.06%)	⬇️
routers/repo/view.go	41.08% <0%> (-1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9fd8b26...d29abcf. Read the comment docs.

[zeripath]

@sapk do you think you can fix the issues you mentioned?

[sapk]

@zeripath Since it is mostly to ease validating UI PR and have some data to view, I didn't take time on that part. I think it is related to fixtures but can't find exactly why.

[lafriks]

@zeripath I think fixing fixtures should be out of scope for this PR

[zeripath]

I was meaning about keeping the user logged in, but if it's too hard to fix then we can ignore it as it's only contrib

[jolheiser]

I couldn't for the life of me figure out how to get it to retain auth, but the rest of these nits just came about as I used the contrib. I don't think any of my nits are blocking though, so I'll just leave them as comments.
It seems to work well enough otherwise. 😄
I will say this makes checking out PRs incredibly easy!

[jolheiser]

After testing this again with latest changes, now it seems to be complaining about unstaged changes, even though I have a clean working tree. Did you run into this at all?

EDIT: After changing the checkout method to

err = tree.Checkout(&git.CheckoutOptions{
	Branch: plumbing.ReferenceName(branch),
	Force: true,
})

it worked...not sure if an issue with my setup or if it is needed for some reason. It shouldn't need to force the reset, but perhaps something else in the code is causing a "dirty" working tree before checkout?

[lafriks]

Would be nice to add shortcut to makefile for this, something like make pr 1234

[sapk]

@jolheiser I did run it but it seems to not really create the branch at checkout but only checkouting the ref.
The unstaged changes can maybe come from the fact that the source file is duplicated in the destination repo if it doesn't exist in this revision. I will look into it.

[sapk]

@jolheiser did you use the command on windows ?
On windows : I have the error message of unstaged changes
On linux : It doesn't complain and checkout the PR and to the rest but it doesn't create the branch that it should at fetch step.

I will debug more to find why.

[sapk]

I find the mistake for the linux but still have the error on windows. The current solution is to use Force: runtime.GOOS == "windows" to only force on windows. (like suggested by @jolheiser)

@lafriks To have a make target like that we should had some hacky code like :

ifeq (pr, $(firstword $(MAKECMDGOALS)))
  runargs := $(wordlist 2, $(words $(MAKECMDGOALS)), $(MAKECMDGOALS))
  $(eval $(runargs):;@true)
endif
.PHONY: pr
pr:
	$(GO) run contrib/pr/checkout.go $(runargs)

Or we could also use format like make pr PR=0000

.PHONY: pr
pr:
    (GO) run contrib/pr/checkout.go $(PR)

[jolheiser]

I had been testing on Windows at the time, yes. Good to know it's resolved on Linux, as that's where I will probably be doing most of my dev work going forward.

The only issue I can think of is if Force would discard working changes the dev wanted but forgot to commit before using the contrib. (Although we have fairly few Windows devs afaik)

As for the Makefile, personally I think the second option looks cleaner, and I like the idea of not making the Makefile any messier.

[lafriks]

I'm ok with second option for makefile