Skip to content

Fix usergroups.users.{list,update} and files.commnets.{add,edit,delete} APIs#82

Closed
masatomo wants to merge 2 commits intoslack-ruby:masterfrom
masatomo:fix-2-dots-apis
Closed

Fix usergroups.users.{list,update} and files.commnets.{add,edit,delete} APIs#82
masatomo wants to merge 2 commits intoslack-ruby:masterfrom
masatomo:fix-2-dots-apis

Conversation

@masatomo
Copy link
Copy Markdown
Contributor

Some Slack APIs has 2 dots like usergroups.users.{list,update} / files.commnets.{add,edit,delete}
These APIs didn't work and this patch fixes it.

Comment thread CHANGELOG.md Outdated
### 0.7.1 (Next)

* Your contribution here.
* [#82](https://github.com/dblock/slack-ruby-client/pull/82): Fix usergroups.users.{list,update} and files.comments.{add,edit,delete} APIs
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this look like the other entries please, quote the methods and add your - name. at the end.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Apr 29, 2016

Do I understand correctly that the implementation here of files_comments is actually not an API, the API is files_comments_edit and such? That makes sense, so the only problem in this PR I think is the command line implementation where you want files comments edit kind of API.

@masatomo masatomo force-pushed the fix-2-dots-apis branch 2 times, most recently from 9d3ecf3 to 9198093 Compare April 29, 2016 18:32
@masatomo
Copy link
Copy Markdown
Contributor Author

Do I understand correctly that the implementation here of files_comments is actually not an API, the API is files_comments_edit and such?

Right. refs. https://api.slack.com/methods/files.comments.edit

Also, I've updated everything you pointed out.

Comment thread CHANGELOG.md Outdated
### 0.7.1 (Next)

* Your contribution here.
* [#82](https://github.com/dblock/slack-ruby-client/pull/82): Fix `usergroups.users.{list,update}` and `files.comments.{add,edit,delete}` APIs - [@masatomo](https://github.com/masatomo)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Missing period at the end.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented Apr 30, 2016

Please check the build, you have to fix rubocop at the very least, run rubocop -a, then rubocop --auto-gen-config for everything else. Squash your commits. Thanks, this is close to be merged.

@masatomo masatomo force-pushed the fix-2-dots-apis branch 2 times, most recently from 2cc86d4 to 2ac31a3 Compare May 2, 2016 04:44
@masatomo
Copy link
Copy Markdown
Contributor Author

masatomo commented May 2, 2016

ok. but seems robocop -a updates many files which I didn't touch for this pull-request. do you still want me to run it?

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented May 2, 2016

It shouldn't but yes, @masatomo, just make the build pass and then we shall see :)

Comment thread CHANGELOG.md Outdated
### 0.7.1 (Next)

* Your contribution here.
* [#82](https://github.com/dblock/slack-ruby-client/pull/82): Fix `usergroups.users.{list,update}` and `files.comments.{add,edit,delete}` APIs. - [@masatomo](https://github.com/masatomo)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The period is in the wrong place, should be at the end of the line :)

@masatomo masatomo force-pushed the fix-2-dots-apis branch from 2ac31a3 to b87308e Compare May 2, 2016 11:26
@masatomo
Copy link
Copy Markdown
Contributor Author

masatomo commented May 2, 2016

seems rubocop verison in trivis-ci is older than mine what do you want me to do?

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented May 2, 2016

It uses what's inside Gemfile.lock, you probably have a global version and would need to run this with bundle install. I'll take care of this, no worries.

@dblock
Copy link
Copy Markdown
Collaborator

dblock commented May 2, 2016

Merged via dblock@90e69ad. Thank you.

@dblock dblock closed this May 2, 2016
@masatomo
Copy link
Copy Markdown
Contributor Author

masatomo commented May 2, 2016

ok, thanks!

dblock referenced this pull request in mooremo/slack-ruby-client May 2, 2016
dblock referenced this pull request in binarylibrarian/slack-ruby-client May 2, 2016
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.

2 participants