-
Notifications
You must be signed in to change notification settings - Fork 263
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
Prepare v4 release #312
Prepare v4 release #312
Conversation
#311 should be merged before we release v4. |
If you're looking at a V4 release, you might drop the default for the V3 |
05d02a4
to
964de73
Compare
Hi @Andrew-Chen-Wang, any chance you are able to review this? |
Thanks for reviewing @Andrew-Chen-Wang! I'll continue preparing the release when I have time. Regarding your comment @alexrobertson-muckrack
I'm not sure what the benefit of this breaking change would be. |
@Stormheg it would require users of V3 ReCAPTCHA to provide individual action values for their forms rather than assume they should all be grouped under the |
Got it, any thoughts on this @Andrew-Chen-Wang? |
It just needs a guide inside of our docs (eg "Migrating to v4" section). Otherwise, many are using the defaults and providing a small hindrance to promote security is beneficial |
What left to be done for release to happen? |
@koutoftimer don't worry, I'll get to this soon. |
I'm not sure, but it looks like closing issue about naming conflict will be beneficial #291 |
Marking as draft pending the renaming of the |
It would be useful to be able to add the action as an attribute (with |
Drop changelog, that could get _long_
Don't pass a default action to the ReCaptcha API anymore. Specifying an action is an optional feature of the ReCaptcha API and thus we should make it optional too by default.
964de73
to
8825cff
Compare
Hey @enzedonline, would you mind creating an issue out of this? From what I understand you want to be able to a specific required score for a specific type of form. This is an interesting idea and I'd love to discuss it further in a separate issue. |
I've rebased and updated the PR with some more changes. Most controversial is dropping the
I think the best way forward is to make actions optional, document it and encourage people to use it. I've added documentation that suggests this, feedback on wording welcome and appreciated! Some more changes include:
|
It is for backwards compatibility as reCaptchav2 is still widely used I believe (yes, even with multi modal LLMs, they're expensive, and any form of captchas/locks are preventable to a certain degree); and imo, v2 is still very good. But this is assuming the backwards compat is actually true. Please let me know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will be busy next few days. I believe everything LGTM (besides the point above which requires further discussion if needed), and if not, I will be available to approve any new review requests immediately since there are a lot of breaking changes and plenty of patches may come ahead.
Thanks for reviewing @Andrew-Chen-Wang 👍 As to to your comment: the reCAPTCHA v2 API does not support actions. It is exclusive to reCAPTCHA v3 (source, Dropping the default action does not introduce any backwards incompatible changes to the RecaptchaV2 widgets. If there are no further comments or concerns I'll get this merged and a release out 🙌 |
For all involved with this PR: I tried to create a release which should trigger the I'm getting in touch with the relevant folks at Torchbox to sort out the PyPI situation as only they have access. |
@Stormheg Sorry, just saw this - added to the ideas list #320 |
I have been entrusted with PyPI access and have set up Trusted Publishing (791f833)
As a result, v4.0.0 is now available on PyPI! I'd like to thank all contributors and @Andrew-Chen-Wang for reviewing my PRs 🚀 |
Reasoning for new major release: we dropped support for unsupported Django and Python versions, which is a breaking change according to SemVer.
I'll fill in the release date once we actually release this.