db, test: do not create nick ID by default #2234
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Having `get_nick_id()` default to `create=True` may cause unexpected behavior if invalid nicknames are passed to it via plugins' use of the getter and setter methods. We are taking this opportunity to fix the API design itself, with no period of deprecation and warnings, since this particular method is mostly used only by the database object itself, and isn't really intended for direct use by plugins. I've also added some more assertions and error-case checking to some of the related tests for `sopel.db`.
dgw
added
Tweak
Breaking Change
dgw
force-pushed
the
no-get_nick_id-create
branch
from
fdb901c to
7d0c5a9
Compare
|
Oh, this will conflict with #2231 a lot. 😁 |
Exirel
approved these changes
Jan 8, 2022
|
Figured if I merge sooner, you can assess + fix the conflicts sooner. 😸 |
|
Is that a permission to rebase right now? 😁 |
|
"As they say in the Temporal Mechanics department, 'There's no time like the present.'" |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Having
get_nick_id()default tocreate=Truemay cause unexpected behavior if invalid nicknames are passed to it via plugins' use of the getter and setter methods.We are taking this opportunity to fix the API design itself, with no period of deprecation and warnings, since this particular method is mostly used only by the database object itself, and isn't really intended for direct use by plugins.
I've also added some more assertions and error-case checking to some of the related tests for
sopel.db. The tests required minimal changes to continue passing; only cases where some manual setup was usingget_nick_id()as a shortcut to make sure the nick would exist in the database before running the test case needed to be changed.Checklist
make qa(runsmake qualityandmake test)Notes
A code search on GitHub for
get_nick_idreturned very few results that weren't either a Sopel plugin or a clone of Sopel itself being used as the base for a custom bot. Of the results that were actually plugin code, only one out of a dozen or so might be problematic (and I will open an issue on that repo to let the author know about this planned change).As I thought, not many people actually use this function directly. At least half the plugin results were from my plugins or modified clones thereof (which already used
create=Falseexplicitly).