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

ip: remove plugin extracted to external package #2523

Merged
merged 1 commit into from
Oct 30, 2023

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Oct 14, 2023

Description

This PR removes the built-in ip plugin in favor of the newly packageized sopel-iplookup plugin, as part of #1291. Note that since #2504 hasn't landed yet, I'm still using the older modules terminology here, but if that PR lands first, this one should be reworked to use the new terminology.

This should not be merged until sopel-iplookup has been uploaded to PyPI. Edit: YOLO merge whenever I guess

I have marked this as a breaking change because, per discussion on IRC, we decided that it makes sense for the new package to name the plugin iplookup rather than the more generic ip. If users mention ip in their bot configuration (i.e. to exclude it), they will need to change the name to account for that.

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 lint and make test)
  • I have tested the functionality of the things this change touches

@SnoopJ SnoopJ added the Breaking Change Stuff that probably should be mentioned in a migration guide label Oct 14, 2023
@SnoopJ SnoopJ mentioned this pull request Oct 14, 2023
7 tasks
pyproject.toml Outdated Show resolved Hide resolved
@dgw dgw added this to the 8.0.0 milestone Oct 14, 2023
@dgw dgw changed the title modules: replace ip plugin with external package modules: extract ip plugin to external package Oct 14, 2023
@dgw dgw changed the title modules: extract ip plugin to external package ip: remove plugin extracted to external package Oct 14, 2023
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.

Derps all sorted out from my end, including the correct title format based on the last half-dozen times we've done this. Just need a squash & commit-message edit.

@SnoopJ SnoopJ force-pushed the feature/use-external-iplookup branch 2 times, most recently from 54e08a1 to 0752ff8 Compare October 15, 2023 17:31
@dgw
Copy link
Member

dgw commented Oct 27, 2023

We've published sopel-iplookup 1.0.0. I had a soft-block on this being merged before publication, despite @SnoopJ's "YOLO" above, but the plugin's up now so :shipit:

@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 27, 2023

Glad we didn't merge yet, we'd missed that this PR didn't drop the geoip2 requirement! With that change added in bd1e6f1, the path to #2516 should be unblocked, as the breakage caused by the upstream troubles in aiohttp will be quarantined to the third-party package.

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Yay let's go!

@dgw
Copy link
Member

dgw commented Oct 28, 2023

I'd ideally like the removal to be a single operation, deleting the plugin and its requirement in one go. I'd also like the Coveralls check statuses to report properly… They're getting stuck on Pending a lot lately and it always seems to be on commits that don't touch any code. 🤔

@SnoopJ SnoopJ force-pushed the feature/use-external-iplookup branch from bd1e6f1 to e4c5166 Compare October 28, 2023 18:27
@SnoopJ
Copy link
Contributor Author

SnoopJ commented Oct 28, 2023

I'd ideally like the removal to be a single operation, deleting the plugin and its requirement in one go.

Rebased, should be the case now.

I'd also like the Coveralls check statuses to report properly

Yea, dunno what's going on with them 🤷

@dgw dgw merged commit 166de9f into sopel-irc:master Oct 30, 2023
13 checks passed
@SnoopJ SnoopJ deleted the feature/use-external-iplookup branch November 5, 2023 20:18
@dgw dgw mentioned this pull request Nov 16, 2023
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Stuff that probably should be mentioned in a migration guide
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants