Skip to content

Conversation

@Exirel
Copy link
Contributor

@Exirel Exirel commented Mar 8, 2021

Description

image

Following the same pattern as url/url_lazy, there is now 3 new decorators:

  • rule_lazy
  • find_lazy
  • search_lazy

I saw 3 things:

  • the logic behind getting regexes for a callable can be factored, so I did that
  • the sopel.plugins.rules is way too big, a future PR will split that in different files (waiting on rules: fix deprecated usage of tools.compile_rule #2034 to be merged first)
  • tests are not up to my usual standards on that, between loaders, rules, and bot, there are stuff missing, and I decided not to overdue it here, that'll be for another PR

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 Mar 8, 2021
@Exirel Exirel added this to the 7.1.0 milestone Mar 8, 2021
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.

Oh look, it's me nitpicking documentation style again. 👀

Annoying that AppVeyor failed the checks even though it's not even active, but I think I fixed that once and for all now. Next push shouldn't trigger it.

@Exirel Exirel requested a review from dgw April 9, 2021 22:21
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.

Good to go. Squash out the fixups and that'll also get rid of the spurious failure ❌ in the PR's commit history. (Despite never being fully set up, AppVeyor was still getting PR notifications, which I believe is now correctly turned off.)

Exirel and others added 7 commits April 24, 2021 09:14
* plugin: add the rule_lazy decorator
* loader: properly clean callable with lazy rule loaders
* plugins.rules: new `*_lazy` classmethods to handle lazy rules
* bot: call `from_callable_lazy` for lazy rules

Also, I realized I could do some refactorization of how rule & URL
callback are created, as they share more in common than I though. This
will be reused later when find_lazy and search_lazy will be added.

Note: I know, big commit, not a huge fan either. The next ones should be
a bit shorter.
@Exirel Exirel force-pushed the plugins-more-rule-lazy branch from 256e716 to 08d0204 Compare April 24, 2021 07:15
@Exirel
Copy link
Contributor Author

Exirel commented Apr 24, 2021

Rebased & squashed.

@dgw
Copy link
Member

dgw commented Apr 25, 2021

Rebased & squashed.

Squashed? I guess technically, but b962741 and 08d0204 still look kinda fixuppy. 👀 But I'm also tired of going around and around on this, so I'll give you a pass. 😹

@dgw dgw merged commit b1b9a19 into sopel-irc:master Apr 25, 2021
@Exirel Exirel deleted the plugins-more-rule-lazy 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.

2 participants