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

Support WTForms 3 #916

Merged
merged 4 commits into from Jan 23, 2022
Merged

Support WTForms 3 #916

merged 4 commits into from Jan 23, 2022

Conversation

Glandos
Copy link
Member

@Glandos Glandos commented Nov 12, 2021

This bring the latest version, but some breaking changes are introduced.
Especially, StringField now default to None instead of '', so I changed the tests, without knowing the real impact.

@zorun do you have an idea of a real use case where it can break? All columns are nullable, as far as I know, but I want to be extra sure.

@Glandos Glandos requested a review from zorun November 12, 2021 22:35
@Glandos
Copy link
Member Author

Glandos commented Nov 12, 2021

Ah, of course, this breaking change cannot be backported, at least in tests… I'll see it later, then.

@@ -35,7 +35,7 @@ install_requires =
Flask-SQLAlchemy>=2.4,<3
Flask-Talisman>=0.8,<1
Flask-WTF>=0.14.3,<2
WTForms>=2.3.1,<2.4
WTForms>=2.3.1,<3.1
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be >=3.0 ?

)

try:
# Compat for WTForms <= 2.3.3
Copy link
Member

Choose a reason for hiding this comment

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

Why should we keep backwards compatibility?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's exactly my question. My experience showed me that jumping early on the "new backward incompatible release" train can be really annoying. So I tend to prefer solutions that introduce a backward compatibility layer, but I think this is not possible here, due to the default change for string values.

Copy link
Member

Choose a reason for hiding this comment

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

My experience showed me that jumping early on the "new backward incompatible release" train can be really annoying.

Ah, that seems bad indeed. Could you explain why?

@symphorien
Copy link

can this be merged ? I kind of need that for the downstream nixos package.

@almet
Copy link
Member

almet commented Jan 23, 2022

Okay, let's do that for now, and drop it later on :-)

@almet almet merged commit 40ce32d into spiral-project:master Jan 23, 2022
@symphorien
Copy link

thanks !

TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
* Support WTForms 3

* default value to None for WTForm backward compatibility

* switch back to empty string as default

WTForm backward compatibility needs that

* format
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.

None yet

3 participants