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

Add Bitrise as a CI provider #483

Merged
merged 1 commit into from
Jan 18, 2018
Merged

Add Bitrise as a CI provider #483

merged 1 commit into from
Jan 18, 2018

Conversation

tychota
Copy link
Contributor

@tychota tychota commented Jan 18, 2018

Hello and thank for the invitation in the organisation :)

In my current project, the client use bitrise as a CI.

Bitrise is supported in danger-rb (https://github.com/danger/danger/blob/ccd0a83e42b9825761abb56d2870877e3ec6447c/lib/danger/ci_source/bitrise.rb) but not in danger-js

I did adapt buildkite provider, added Tests and make them pass, in regards to the ruby implementation

My only uncertainty is https://github.com/danger/danger-js/compare/feature/provider/bitrise?expand=1#diff-57355ea59dd912ef267ff074f7ddf8a4R38 as it is used https://github.com/danger/danger/blob/ccd0a83e42b9825761abb56d2870877e3ec6447c/lib/danger/ci_source/bitrise.rb#L24 but not documented here http://devcenter.bitrise.io/faq/available-environment-variables/.

--EDIT START--
They confirm that BITRISE_IO is true when runned on Bitrise https://twitter.com/bitrise/status/953984609531899904
--EDIT END--

If you need any changes feel free add as much stuff as you want on the PR or to ping me on twitter (@tychota).

@viktorbenei
Copy link

@DangerCI
Copy link

DangerCI commented Jan 18, 2018

Warnings
⚠️

Please add a changelog entry for your changes.

Generated by 🚫 dangerJS

@tychota
Copy link
Contributor Author

tychota commented Jan 18, 2018

@orta

For:
image

what are exactly the the steps ?

Shall I:

  • manually bump the version from 3.0.5 to 3.1.0 (as it adds a functionality in a backward compatible manner)
  • add a new section in changelog ?
  • add a text like Support bitrise as a CI provider 🍾 - @tychota

Or shall i just add the text to changelog between "Master" and "3.0.5" and let you bump the version next time you deploy ?

I think it can be useful for new contributors like me to update https://github.com/danger/danger-js/blob/master/CONTRIBUTING.md with these information.

@orta
Copy link
Member

orta commented Jan 18, 2018

The CHANGELOG.md file does request adding it into the master section, I'm happy to have that expanded if that isn't enough (or have the danger rule be more verbose too ) 👍

Anyway, I've re-ran this and it all looks good. When it greens I'll ship a build

@orta
Copy link
Member

orta commented Jan 18, 2018

Eh, I'm just gonna take this and make a PR for #484

@orta orta merged commit 2cf5d11 into master Jan 18, 2018
@orta orta deleted the feature/provider/bitrise branch January 18, 2018 21:46
tychota added a commit that referenced this pull request Jan 18, 2018
After this PR: #483, I did not upgrade the changelog.
@tychota tychota mentioned this pull request Jan 18, 2018
@tychota
Copy link
Contributor Author

tychota commented Jan 18, 2018

Thank you for merging this.
I created a PR to upgrade the changelog.

I think it won't hurt to add a small section in contributing.md and change the danger message from "Please add a changelog entry for your changes." to "Please add a changelog entry for your changes. To do this, add your changes between 'master' and the current version in changelog.md".

I will do a PR for this tomorrow or this week end.

@peril-staging
Copy link
Contributor

peril-staging bot commented Jan 18, 2018

Thanks for the PR @tychota.

This PR has been shipped in v3.0.6 - CHANGELOG.

@viktorbenei
Copy link

@orta the @danger-in-peril CHANGELOG link is 404 (#483 (comment))

@viktorbenei
Copy link

Thanks @tychota for the PR / adding bitrise.io support! 🎉 :)

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 this pull request may close these issues.

None yet

4 participants