hipchat / hipchat-rb

HipChat HTTP API Wrapper in Ruby with Capistrano hooks
https://www.hipchat.com/docs/apiv2
MIT License
336 stars 172 forks source link

Added ability to update user presence #181

Closed craig-gomes closed 7 years ago

craig-gomes commented 7 years ago

I added the ability to update the user status and message. The test coverage for my code is minimal at best, but I wanted to get this PR submitted while I investigate and possibly get some feedback.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling d1143bf5c3ed2d43cfe519097c5b62992fc3b25f on craig-gomes:master into e65b02d966ebce30d0601a3235fa6e6e9cef3216 on hipchat:master.

craig-gomes commented 7 years ago

@coveralls bad choice of words. The test code I wrote was minimal.

Looking into travis failure.

gabrieldeal commented 7 years ago

I just started working on adding this same functionality. Fortunately I didn't get too far before I noticed this PR. :)

I submitted PR #182 about the dependency problem that is causing the Travis failure.

craig-gomes commented 7 years ago

Added changes to include all of the fields for the update user API - https://www.hipchat.com/docs/apiv2/method/update_user

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling d3d31f11d45dc9f352c2a73bfc7d267875f5f5d0 on craig-gomes:master into e65b02d966ebce30d0601a3235fa6e6e9cef3216 on hipchat:master.

gabrieldeal commented 7 years ago

Dear maintainers, I'm curious why there has been no feedback on this PR. Did I muddy the waters by adding my own feedback? Is there no interest in this feature? No time to look at it right now? Or is it that the new code doesn't adhere to the conventions of the existing code?

If the last reason, then I particularly want to know because I would be happy to work with @craig-gomes on adhering to the conventions or I would also be happy to finish the implementation that I started before I found this PR (https://github.com/gabrielmdeal/hipchat-rb/commit/f3d777343e5d93679feabfa5f354d51f3712626b).

rberrelleza commented 7 years ago

Apologies @gabrielmdeal , I was on a holiday and didn't got a chance to look at the PRs until this week (feelsbadman)

craig-gomes commented 7 years ago

I updated the simple changes but not the testing. I'm still learning rspec. I will tinker with the code I've seen in PR #187 around testing as it may serve the same purpose here. Any help on the right way to test this would be appreciated.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 991ee552806e989a31b82c75c26cb486fd3ef941 on craig-gomes:master into e65b02d966ebce30d0601a3235fa6e6e9cef3216 on hipchat:master.

gabrieldeal commented 7 years ago

@craig-gomes I'd be happy to add some expectations to the spec. I'll put it on my calendar for the weekend.

gabrieldeal commented 7 years ago

@craig-gomes FYI, I just submitted a PR (https://github.com/craig-gomes/hipchat-rb/pull/1) against your fork of hipchat-rb.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 448ab0d4b2c2f5c44e3da09a7eba4f5d85c58d19 on craig-gomes:master into e65b02d966ebce30d0601a3235fa6e6e9cef3216 on hipchat:master.

jmgarnier commented 7 years ago

This look like a nice feature 👍 🌹

@rberrelleza do you any plan to merge it? I will just use https://github.com/craig-gomes/hipchat-rb/ for the time being