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

Remove HipChat#developers and ChatClient#developers. #13430

Merged
merged 3 commits into from Feb 24, 2017

Conversation

ashercodeorg
Copy link
Contributor

An alternate approach would be to rename developers to message_server_operations. However, this seems inferior, as it encourages a proliferation of such aliases.

@ashercodeorg ashercodeorg changed the title Remove usage of HipChat#developers. Delete ChatClient#developers. Remove HipChat#developers and ChatClient#developers. Feb 24, 2017
Copy link
Contributor

@aoby aoby left a comment

Choose a reason for hiding this comment

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

It does seem a little cumbersome and prone to misspellings to pass 'server operations' to every call. Since all the messages are currently going there, perhaps we should set that as the default room and make room an optional named parameter?

Then the common call (every one currently) would be simply ChatClient.message 'my message', and we wouldn't need any aliases.

@ashercodeorg
Copy link
Contributor Author

Not all the messages are going there, we use lots of other rooms in HipChat#message. That said, I like the suggestion of a default room, let me do that.

@ashercodeorg
Copy link
Contributor Author

For convenience, also as it seems tangential to resolving the TODO, I'm going to do the default room in a separate PR. Also, that way I can add the default room after removing users of HipChat (would prefer to not need to make that change twice). And I can do so after adding tests to ChatClient and Slack.

@ashercodeorg ashercodeorg merged commit f2a34f5 into staging Feb 24, 2017
@ashercodeorg ashercodeorg deleted the chatClientDevelopers branch February 24, 2017 16:20
aoby added a commit that referenced this pull request Feb 24, 2017
…elopers

Remove leftover call to nonexistent method (removed in #13430)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants