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

Passing keyword only args as positional should fail. #406

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ianjosephwilson
Copy link

Issue

#405

This is a backwards incompatible change because code that was "working" will now throw an exception.

Historically related

836e5f9

@zzzeek zzzeek requested a review from sqla-tester June 16, 2024 00:01
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision c8acb9a of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change c8acb9a: https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5337

@zzzeek
Copy link
Member

zzzeek commented Jun 16, 2024

tricky, because extremely trivial, yet the backwards compat for a very slow moving project like Mako implies we'd need a major release.

waht if we just keep this as an unusual idiosyncratic behavior and just add a test that it passes?

@ianjosephwilson
Copy link
Author

That's fine. Someone just asked about on stackoverflow. Should there be a note in the Using defs part of the documentation or just leave the test as the documentation?

@zzzeek
Copy link
Member

zzzeek commented Jun 17, 2024

I think adding a doc note is fine, indicating there's a bit of legacy behavior here to be aware of, in addition to a test that notes it's related to issue #405

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants