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

irc.isupport, announce: fix unexpected error (get) from .announce #2048

Merged
merged 3 commits into from
Apr 22, 2021

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Mar 28, 2021

Description

Fix #2040

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 Feature Medium Priority Bugfix Generally, PRs that reference (and fix) one or more issue(s) labels Mar 28, 2021
@Exirel Exirel added this to the 7.1.0 milestone Mar 28, 2021
@Exirel Exirel requested a review from a team March 28, 2021 09:39
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.

I have a confuse.

sopel/modules/announce.py Outdated Show resolved Hide resolved
@Exirel
Copy link
Contributor Author

Exirel commented Apr 9, 2021

Design consideration aside (that we should totally start to address in 8.0), I think @dgw is right that the documentation is not good enough. In an attempt to improve the situation with a small increment, I pushed a new commit with this:

image

It's not enough, I think a note should be added about the differences between internal representation and attributes, and we should probably give an explanation of how the isupport parser system works—even though it's all pretty internal stuff in here, and nothing will replace a good knowledge of IRC spec.

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.

I've come to terms with how this interface works for 7.x, plus both of us are probably ready to move on until we start on 8.0. The documentation side might be more helpful if it placed more emphasis on interacting with bot.isupport using getattr()? But that's for a separate PR, and likely 8.0.

@dgw dgw merged commit 0608a9e into sopel-irc:master Apr 22, 2021
@Exirel Exirel deleted the fix-announce 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
Bugfix Generally, PRs that reference (and fix) one or more issue(s) Feature Medium Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected error (get) from .announce
2 participants