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

Eliminate remaining client.VERSION{,_STRING} calls #175

Closed
wants to merge 1 commit into from
Closed

Eliminate remaining client.VERSION{,_STRING} calls #175

wants to merge 1 commit into from

Conversation

fungi
Copy link

@fungi fungi commented Aug 11, 2020

Commit 313620f removed irc.client.VERSION and
irc.client.VERSION_STRING while they were still used in irc.bot,
irc.server and unit testing. Get rid of those remaining instances,
and also remove the unit test routine which checked for the presence
of these variables since the module is no longer responsible for
them. Further clean up an unused reference to importlib_metadata in
the setup.cfg.

Fixes #174

@fungi
Copy link
Author

fungi commented Aug 11, 2020

Oh, yep, I clearly only tested this on Python 3.8, importlib.metadata isn't present in stdlib prior to that. Also looking closer at commit 313620f I suspect the real goal there was to get rid of calls to importlib itself, so all I'm doing is reintroducing them elsewhere. I suppose what needs determining is whether the actual library version needs to be returned for CTCP VERSION requests in irc.bot.SingleServerIRCBot.on_ctcp() or if it should just make something up?

I'll push up a revision to see what folks think about returning the Python interpreter version instead.

Commit 313620f removed irc.client.VERSION and
irc.client.VERSION_STRING while they were still used in irc.bot,
irc.server and unit testing. Get rid of those remaining instances,
and also remove the unit test routine which checked for the presence
of these variables since the module is no longer responsible for
them. Further clean up an unused reference to importlib_metadata in
the setup.cfg.

Fixes #174
@Foxboron
Copy link

This broke an Arch Linux irc bot :)

@fungi
Copy link
Author

fungi commented Aug 27, 2020

This broke an Arch Linux irc bot :)

Yes, unsurprisingly what prompted me to push this PR up is that it broke an IRC bot I help run (in this case a 100+ channel service heavily relied on by the OpenStack community).

@eepp
Copy link

eepp commented Sep 4, 2020

When is the next patch release including this?

@jaraco
Copy link
Owner

jaraco commented Sep 8, 2020

Thanks for this, but I do believe the library benefits from advertising its own version, so I'll provide an alternate fix.

@jaraco jaraco closed this Sep 8, 2020
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.

The internals still rely on VERSION_STRING
4 participants