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

tools: new SopelIdentifierMemory class #1938

Merged
merged 2 commits into from
Sep 26, 2020

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Sep 12, 2020

Description

In an attempt to fix #1341, I decided to create a new class that specifically deals with tools.Identifier as its keys: nothing more, nothing less, just Identifier.

My idea is that we want SopelMemory to consider "nick" and Identifier("nick") as two separated keys, and what we need is a specific type of memory, one that is specifically designed to store and retrieve values from Identifier key. So that's what I did here.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added the Feature label Sep 12, 2020
@Exirel Exirel added this to the 7.1.0 milestone Sep 12, 2020
@Exirel Exirel requested a review from dgw September 12, 2020 17:10
@Exirel Exirel force-pushed the tools-memory-key-factory branch from 959db6f to d451590 Compare September 12, 2020 17:12
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

Those tests you added in tools have disabused me of any remaining notion that you're a humble man. 😛 Just one doc nitpick and a rebase. If you want to fix d451590's commit message so it capitalizes the new class name correctly, that's great too.

sopel/tools/__init__.py Outdated Show resolved Hide resolved
@Exirel Exirel force-pushed the tools-memory-key-factory branch from b3ecb62 to 4374831 Compare September 23, 2020 07:05
@Exirel Exirel force-pushed the tools-memory-key-factory branch from 4374831 to 8943564 Compare September 23, 2020 07:08
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

This is a nice, simple addition. Honestly can't wait to make use of it in my own plugins, too.

@dgw dgw merged commit 54e4274 into sopel-irc:master Sep 26, 2020
@Exirel Exirel deleted the tools-memory-key-factory branch May 1, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent SopelMemory behavior with Identifier keys & lookups by string
2 participants