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

feat: hipchat integration #320

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Conversation

salzig
Copy link
Contributor

@salzig salzig commented Oct 4, 2016

HipChat Integration \o/

@salzig salzig force-pushed the feat/hipchat branch 2 times, most recently from 90f9d7d to 75b975e Compare October 4, 2016 18:40
@salzig
Copy link
Contributor Author

salzig commented Oct 5, 2016

There are some errors unrelated to my change.

First with 1.9.3, which should break every newer build.

Gem::InstallError: term-ansicolor requires Ruby version >= 2.0.
Installing simplecov 0.12.0
An error occurred while installing term-ansicolor (1.4.0), and Bundler cannot

And then 2.0.0 and 2.3.0 which is maybe just a timing problem? At least i can't see how my change could produce this problem only with 2.0.0 and 2.3.0, but not with 2.1.7 and 2.2.4.

expected: 1
     got: 0
(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)
./features/step_definitions/lolcommits_steps.rb:160:in `/^there should be exactly (.*?) (jpg|gif|pid)s? in "(.*?)"$/'
features/lolcommits.feature:74:in `Then there should be exactly 1 pid in "~/.lolcommits/forked"'
Failing Scenarios:
cucumber features/lolcommits.feature:70 # Scenario: Capture doesnt break in forked mode

@mroth, @matthutchinson any opinion?

@matthutchinson
Copy link
Member

Sorry for the later reply - i'll look into the 1.9.3 issue now. The 'forked mode' pid tests are known to be a bit flakey and should pass on a re-run.

@matthutchinson
Copy link
Member

This PR should fix the 1.9.3 issue (and another with 2.1) - I'll merge it when green - I restarted your other failed builds with the forked issue and they passed OK

You can then rebase with upstream/master and your PR should pass.

@salzig
Copy link
Contributor Author

salzig commented Oct 9, 2016

@matthutchinson done :)

@matthutchinson
Copy link
Member

You'll need to add a require for this new file, in lib/lolcommits.rb along with the other plugins. Other than that, this looks good to me.

@salzig
Copy link
Contributor Author

salzig commented Oct 10, 2016

@matthutchinson good point, changed that.

@matthutchinson
Copy link
Member

Great merging now 👍

@matthutchinson matthutchinson merged commit cff6255 into lolcommits:master Oct 10, 2016
@salzig salzig deleted the feat/hipchat branch October 10, 2016 13:28
@matthutchinson
Copy link
Member

Hi @salzig 👋

So you may (or may not) have noticed that we recently extracted all lolcommit plugins to external gems!

🏅 Since you contributed this hipchat plugin, I've added you (as an admin) to the new lolcommits-hipchat repo.

Of course you are under no obligation to maintain this code or gem going forward. This is just a quick message to explain what has happened and give you the opportunity to take ownership again.

If you're interested in picking this up, send me your email address (to matt/at/hiddeloop.com) and I'll add you as a new gem owner (with publishing rights) here.

Otherwise I will continue to maintain the gem, and make sure things are kept up to date.


To get an idea of how gem plugins work, check out the sample_plugin README / RubyDocs or browse through other plugin code.

The main reasons for extraction were to:

  • remove the ever-growing gem dependency list in the main lolcommits gem
  • start to unify configuration code for plugins (common helpers, approaches to OAuth etc.)
  • make lolcommit plugin development easier
  • add more comprehensive plugin tests

That's it, any questions just let me know.

Happy coding!

Matt

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

2 participants