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

Fixing #220 (by reverting the breaking change) #222

Closed
wants to merge 1 commit into from

Conversation

violuke
Copy link

@violuke violuke commented Oct 19, 2022

Please see #220, this is causing a lot of problems as a breaking change was accidentally introduced in 2.4.

It might be that there is a "better" fix in #221 or via another solution, but I think the priority should be to release 2.4.1 with this PR in and the original functionality fixed.

This would buy time to find a better solution that allows for the request of #168 to be implemented without causing the problems that the original change caused.

Thank you for this fantastic package.

@spapas
Copy link

spapas commented Oct 19, 2022

Since there already is a solution of the problem (#221) I think the correct approach would be to go with that solution instead of reverting the change (which was on the correct direction). Peopl that are experiencing problems should not upgrade to 2.4 for now.

@schinckel
Copy link
Contributor

schinckel commented Nov 22, 2022

I'm going to recommend merging this, because I don't have the time to commit to a proper solution (and it's very likely that nobody else is using the feature other than me at this stage).

@robd003
Copy link

robd003 commented Dec 19, 2022

Any chance this can get merged in and cut a 2.4.1 release?

@schinckel @violuke

@schinckel
Copy link
Contributor

I can't merge PRs.

@violuke
Copy link
Author

violuke commented Dec 19, 2022

I'd also love this merged, but I also cannot merge PRs. @claudep, @felixxm and @hramezani can you help?

@hramezani
Copy link
Member

Sorry for the late response. I am not very familiar with the project TBH.

Agree that the fix is not a proper fix and cause some problem.
Reverting the fixing might not be good because the other people who are using the feature might be hurt.
I would suggest finding a better solution and meanwhile, you can pin the version to 2.3

Is there any important feature in 2.4 that this bug blocks you to upgrade and use it?

@spapas
Copy link

spapas commented Dec 21, 2022

I would agree with @hramezani on this; releasing a new version without a proper fix would result to people that are using the new behavior having problems! In the previous version the bug was slipped under the rag and was introduced without anybody noticing.

Also, please notice that if you want to upgrade and have the old behavior you can simply override the get_form method of you WizardView to use self.form_list like already proposed in this comment:

#220 (comment)

I know this is not ideal however it should allow you to upgrade until #220 is actually fixed.

@claudep
Copy link
Contributor

claudep commented May 13, 2023

Thanks for the patch, however hopefully 604a8ec should fix the regression.

@claudep claudep closed this May 13, 2023
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

6 participants