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

cli: fix legacy parser options for Py3.9.8 (Sopel 8.x) #2212

Closed
wants to merge 2 commits into from

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Nov 10, 2021

Description

Same as #2211 but for the current development version (8.x). I hope Github Action will pass!

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

The hack adds a prefix to the "dest" of the legacy options added to the
main parser. It works because these options are not actually used by the
legacy subparser: they exists only to trick the help message into
something usable by end-users. It's rather ugly, but at this point we
already know that the "legacy" subparser trick is ugly.

We are only fixing the issue introduced by Python 3.9.8 that "fixed"
something in argparse that broke our use-case. Let's not delve into
that for now.
@Exirel Exirel added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Nov 10, 2021
@Exirel Exirel added this to the 8.0.0 milestone Nov 10, 2021
@dgw
Copy link
Member

dgw commented Nov 11, 2021

FWIW, the patch to revert the change that makes this patch necessary is awaiting merge and backporting. python/cpython#29525

@dgw
Copy link
Member

dgw commented Nov 29, 2021

Open to suggestions for why we should do this anyway, even though Python 3.9.9 reverted the responsible change.

If the consensus is to skip this "hack" and wait for further news (my inclination), that's the reason for which I've left #2210 open.

@Exirel
Copy link
Contributor Author

Exirel commented Dec 1, 2021

Given that most of the parser will be reworked to remove deprecated options with a better "default action" system... I think we can close this and I'll make sure to include that question into the new version.

@Exirel Exirel closed this Dec 1, 2021
@dgw dgw removed this from the 8.0.0 milestone Dec 1, 2021
@dgw dgw added the Declined Requests that will not be implemented for technical or project direction reasons label Dec 1, 2021
@Exirel Exirel deleted the cli-legacy-options-fix-8.x branch January 20, 2022 07:33
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) Declined Requests that will not be implemented for technical or project direction reasons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants