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

Set CI_BRANCH_BASE for GitHub Actions #114

Closed
XhmikosR opened this issue Apr 8, 2020 · 13 comments · Fixed by #129
Closed

Set CI_BRANCH_BASE for GitHub Actions #114

XhmikosR opened this issue Apr 8, 2020 · 13 comments · Fixed by #129

Comments

@XhmikosR
Copy link
Contributor

XhmikosR commented Apr 8, 2020

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

There's no way to diff against a branch other than master on GitHub Actions CI

If the current behavior is a bug, please provide the steps to reproduce.

https://github.com/twbs/bootstrap/tree/v4-dev

What is the expected behavior?

If this is a feature request, what is motivation or use case for changing the behavior?

Please mention other relevant information such as the browser version, Node.js version, bundlewatch version, and Operating System.

Recently I switched bootstrap from bundlesize to bundlewatch and it works quite well!

I wanted to figure out how to diff against a branch other than master, in our case v4-dev. I couldn't find how to make this work with GitHub Actions, since the env variables don't seem to provide a similar variable to CI_BRANCH_BASE

Does anybody have any ideas?

@XhmikosR
Copy link
Contributor Author

After talking to GitHub support and after doing some more searching it appears GitHub sets both github.head_ref and github.base_ref on pull requests.

This could simplify things in

// GitHub only provides ref (read more: https://stackoverflow.com/q/1526471)
repoCurrentBranch = env.GITHUB_REF.replace(/^refs\/heads\//, '')

...and also allow to use set repoBranchBase.

I don't know if this will fix my issue with our v4-dev branch where bundlewatch is always comparing against master. Maybe I need to set trackBranches only to v4-dev in the v4-dev branch?

@iamogbz
Copy link
Member

iamogbz commented Apr 26, 2020

Looks like github.head_ref and github.base_ref are only set on forked repos https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables

However you should be able to manually set the CI_BRANCH_BASE environment variable to the branch you want to compare against.

@XhmikosR
Copy link
Contributor Author

@iamogbz thanks for the reply!

I can set the environment variable for the v4-dev branch, but since GitHub does provide a way to do this automatically, the code should take this into account, like it does with Travis and the other CI systems.

If I set the environment variable do I also need to change the trackBranches config not to include master in the v4-dev branch?

@XhmikosR
Copy link
Contributor Author

Setting the var works. I didn't change trackBranches.

Anyway, I still think it'd be a nice improvement if bundlewatch detected this automatically like it happens with Travis.

@iamogbz
Copy link
Member

iamogbz commented Apr 27, 2020

@XhmikosR glad it worked for now. Will look into getting to work for github actions as well as other CI.
Open to PRs as well.

@iamogbz
Copy link
Member

iamogbz commented Apr 28, 2020

Will replace the getCIVars with npm

@XhmikosR
Copy link
Contributor Author

I think it's just a couple of lines to add in the current code, though? Don't get me wrong, I also don't like reinventing the wheel, but in the meantime this should do the job 🙂

@XhmikosR
Copy link
Contributor Author

XhmikosR commented May 9, 2020

@iamogbz so, I tried this without setting the CI_BRANCH_BASE variable for a branch based on our v4-dev, but the comparison is wrong, i.e. bundlewatch compares with master. I will add back the variable, I just wanted to let you know, because the issue was closed.

@iamogbz
Copy link
Member

iamogbz commented May 14, 2020

@XhmikosR strange! Do you mind providing more details on your ci logs in the case you're able to reproduce.

@XhmikosR
Copy link
Contributor Author

@iamogbz it seems it's actually broken completely now even for master, but I'm not sure if something else changed: https://github.com/twbs/bootstrap/pull/30814/checks?check_run_id=673413078#step:8:11

For v4-dev without specifying the env var: https://github.com/twbs/bootstrap/runs/659104517?check_suite_focus=true#step:16:1

@iamogbz
Copy link
Member

iamogbz commented May 14, 2020

@XhmikosR

For v4-dev without specifying the env var: twbs/bootstrap/runs/659104517?check_suite_focus=true#step:16:1

Do you have a pull request for that? Since github only set the env vars for PRs

@iamogbz
Copy link
Member

iamogbz commented May 14, 2020

@XhmikosR

it seems it's actually broken completely now even for master, but I'm not sure if something else changed: #30814 (checks)

Caused by dependency update in bundlewatch service resolved now

@XhmikosR
Copy link
Contributor Author

@iamogbz that was in a PR against the v4-dev branch, but I dropped the patch after I noticed it didn't work.

Caused by dependency update in bundlewatch service resolved now

Thanks!

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 a pull request may close this issue.

2 participants