Fix fork PR builds

[benoitdion]

Remove hardcoded reference to the origin remote and instead use the commit sha provided by the github action context.

Taking some of the learnings from #101 and applying them in separate PR's

[codecov]

Codecov Report

Merging #105 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   94.67%   94.69%   +0.02%     
==========================================
  Files          14       14              
  Lines         244      245       +1     
  Branches       48       47       -1     
==========================================
+ Hits          231      232       +1     
  Misses         13       13              

Impacted Files	Coverage Δ
src/model/versioning.js	100.00% <100.00%> (ø)

[benoitdion]

Not sure why the build isn't passing. Not getting any git diffs when running the build locally (node 12)

Edit: This has now been resolved.

[webbertakken]

Yea we'll have to verify this for:

• push to same repo
• pull request to same repo
• pull request to upstream (this repo)
• tag

I had used exactly that command before, but from what I remember it didn't work for all cases. Either i changed something somewhere else and this now works, or this will indeed not work for all above cases.

I will help out with this when I get some time to put in.

[benoitdion]

hey @webbertakken, I have #106 and #101 chained on this PR. What do you think about creating an add-android and android-versioncode branches on your remote? If the branch exists on origin/${branch}, we could decouple the android build work I'm doing from fixing fork PR builds.

The other 2 PRs will be ready for review once I can remove the "fix fork PR builds" commit.

[webbertakken]

Hey @benoitdion

I think it's a good option but I'd rather keep fast forwarding as much as possible to keep forward momentum. All we have to do for this PR is properly test a build for each mentioned scenario using unity-builder@<commit-hash>.

I didn't get to this yet, because I need a moment where I can sit down with enough mindspace to also tackle it if it doesn't work, which i suspect it will for the cases of detached head, where my previously implemented attempt to reattach head may not always be sufficient. I estimate that 2 out of 4 use cases will not work like this. And since we chose for an approach that is supposed to make this uniform for all use cases (which i'm very happy about), we do need to test these properly.

[benoitdion]

I think it's a good option but I'd rather keep fast forwarding as much as possible

I can make sure the git history in the PR is clean so it's not an issue. I'd love to keep the momentum going on the android related PRs.

All we have to do for this PR is properly test a build for each mentioned scenario using unity-builder@

That's great, I didn't realize that worked out of the box.

[benoitdion]

@webbertakken also for this PR, switched approach slightly to use the git commit sha provided by the github action context. I ran into issues with ref when testing this change with my own builds and using the sha directly seems more reliable already.

[webbertakken]

@webbertakken also for this PR, switched approach slightly to use the git commit sha provided by the github action context. I ran into issues with ref when testing this change with my own builds and using the sha directly seems more reliable already.

Awesome, that might have been exactly what I've been doing wrong all this time.

[webbertakken]

Test results:

• on push: https://github.com/webbertakken/unity-builder/runs/789680872
• on internal pr https://github.com/webbertakken/unity-builder/actions/runs/141363350
• on external pr (this pr's latest pipeline will serve as proof)
• amount of commits since latest tag looks correct too

Not tested

• on tag

The tag part has to be tested before creating a new release still. For now i'm merging this to master.

Awesome work @benoitdion