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

Version upgrade #34

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

panatale1
Copy link

Updates for bringing django-superform into modern supported Django and Python

Pete Natale and others added 3 commits September 5, 2024 16:10
@@ -0,0 +1,48 @@
[project]
name = "django-superform4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you explicitly want to change the project name?

I am asking since this is only about adding support for recent Python and Django versions and not about an architectural change in the project.

Choose a reason for hiding this comment

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

My recommendation would be to stick with the same project name.

@@ -34,14 +34,15 @@ def test_it_is_nonzero_for_empty_formsets(self):
form = AccountForm()
bf = form["emails"]
self.assertTrue(isinstance(bf, CompositeBoundField))
self.assertEqual(len(bf), 0)
self.assertIsNone(bf.initial)
# self.assertEqual(len(bf), 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is commented out code. I would suggest removing it.

Suggested change
# self.assertEqual(len(bf), 0)

@gregmuellegger
Copy link
Collaborator

I haven't tested anything locally on my machine. The changes look good so far!
I guess it would be worth making the tests green before merging this into master.

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