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 Discord channel ID matching (PR #187) and tests for it #202

Merged
merged 6 commits into from
Mar 29, 2017

Conversation

Throne3d
Copy link
Collaborator

Builds on PR #187 (credit to @mraof for adding in the actual change to the code). This adds tests, where I think they're supposed to be? It doesn't actually test that Discord – or the library you're using to interface with it – supports IDs in its API, but this seems to work when I use them in my own config, and these tests should ensure the bot's code itself has no trouble with the IDs.

(IDs can be retrieved by writing, e.g., \#general, which should produce something of the form <#00000000>. Remove the <# and > and use that in the config.)

Commit 2ae9e37 was added since the previous check seems to break with this new addition to the config file – it seems to be related to how the items are ordered in the hash and so how they get added to the channels list, which I'm pretty sure isn't important anywhere, so I modified it to a check for presence instead of a check that the first item in the list is precisely that.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.351% when pulling 91bbbad on Throne3d:add-channelids-tests into 26626e5 on reactiflux:master.

@ekmartin
Copy link
Member

Awesome! Thanks @mraof and @Throne3d.

@ekmartin ekmartin merged commit 8c37393 into reactiflux:master Mar 29, 2017
@Throne3d Throne3d deleted the add-channelids-tests branch March 29, 2017 20:24
@ekmartin
Copy link
Member

ekmartin commented Apr 4, 2017

Released in 2.3.0!

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.

4 participants