-
-
Notifications
You must be signed in to change notification settings - Fork 180
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
Gitlab CI support #180
base: master
Are you sure you want to change the base?
Gitlab CI support #180
Conversation
@siddharthkp Have a problem with adding this ? I think this a great fix for now |
Yeah, just the one tiny comment here and in ci-env |
Let me know the comments @siddharthkp. Would be happy to make them quickly. |
src/api.js
Outdated
@@ -12,11 +12,13 @@ let enabled = false | |||
|
|||
if (repo && token) enabled = true | |||
else if (ci) { | |||
warn(`github token not found | |||
if (ci !== 'gitlab') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why specifically gitlab?
CI === github makes more sense to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually ci
variable never takes the value as github
its values are travis, circle etc
(referring to this file). This is because while github doesn't have a platform CI, gitlab it has CI also built into it. Please correct me in case I have understood it wrong.
Might have to to introduce a platform
variable in ci-env
module which would denote the platform on which the code is hosted and based on whether its value is github
or gitlab
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding plaform
makes the most sense to me 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I shall make that change soon 😊
I always forget to hit submit on my review comments 🙈 |
@siddharthkp @SaraVieira I am done with my changes here and in |
@siddharthkp merge?*😊
On Sun, 5 Nov 2017, 13:03 Swapnil Mishra, ***@***.***> wrote:
@siddharthkp <https://github.com/siddharthkp> @SaraVieira
<https://github.com/saravieira> I am done with my changes here and in
ci-env PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#180 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABALddH9L6QtlSEzMFze4IOttz2aUly1ks5szbI6gaJpZM4QR2v->
.
--
Thank you
Sara Vieira
|
@siddharthkp This looks ready to be merged. Are we waiting on any changes? |
🙏🙏🙏 |
Depends on siddharthkp/ci-env#10 |
siddharthkp/ci-env#10 is merged 🚀 |
Description
For now added a check for gitlab so that the message which shows up for github doesn't show up in gitlab CI. Please see screenshots attached for the difference.
I wanted to make few other changes also so that few code paths which are still getting executed, shouldn't but that requires quite a bit of refactoring.
Gitlab has its own CI so we don't need to set the build status explicitly as it's done for other cases.
Motivation and Context
This PR along with siddharthkp/ci-env#10 tries to address #19.
Screenshots (if appropriate):
Given below is sample project which was used to just run gitlab CI and use bundlesize.
Repo: https://gitlab.com/swapnilmishra/test-ci
Failed CI
Passing CI
Message which is shown for github in above screenshots will not be shown for gitlab after the PR.
Types of changes
Checklist: