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

Restore 5.0.x compatibility #63

Merged
merged 28 commits into from
Jul 23, 2019
Merged

Restore 5.0.x compatibility #63

merged 28 commits into from
Jul 23, 2019

Conversation

djay
Copy link
Member

@djay djay commented Jun 27, 2019

No description provided.

@thet
Copy link
Member

thet commented Jun 28, 2019

@djay tnx for working on a plone 5.0 compatibility.

I don't want to merge that into master but use the 2.x branch for that.
That makes it way easier to develop new features, otherwise we'd always have to develop for 5.0 compatibility too. Also the last 5.0 compat approach broke things for 5.1.
Keeping the 2.x and master in sync should be easier than maintaining everything in the master branch.

Are you OK with that?

Any other opinions? @agitator @petschki

@djay
Copy link
Member Author

djay commented Jul 1, 2019

@thet Having a fork makes it a lot harder to fix bugs in your code which is the problem we are facing now

@agitator
Copy link
Member

agitator commented Jul 1, 2019

I'm with @thet here and btw. shouldn't you get rid of py2.7 and update to 5.2 soonish? ;-)

@petschki
Copy link
Member

petschki commented Jul 1, 2019

@djay we use a branch in this repo and not a fork. This is a common method of keeping backward compatibility in many addons of plone.

@agitator nope ... python 2.7 will be there a long time after EOL ... at least until I've merged all my projects to py3 😋

@djay
Copy link
Member Author

djay commented Jul 1, 2019

@petschki not everyone has rich clients willing to pay for the upgrade to an as yet unreleased plone version. Please be pragmatic. We are contributing to this work too. Have some consideration for your collaborators.

@instification instification added this to the Sprint 24 Jun - 12 Jul milestone Jul 8, 2019
@djay
Copy link
Member Author

djay commented Jul 19, 2019

@petschki @thet I think there is a good argument to merge this as it adds useful functionality: namely the ability for the themer or site admin to turn off AJAX loading if they choose. I can see it being problematic for some themes. The reason I like c.collectionfilter over eea.facetednavigation is because it's not making my theming effort harder and is a more default plone experience. Forcing AJAX loading as a compulsory thing I think is the wrong direction.
Other than turning off AJAX loading the changes needed are minimal. There are some enhancements to the relateditems widget we can't use in 5.0 but it's a progressive enhancement... it works ok without them and in fact by putting in a default this version is now better than master.

@petschki
Copy link
Member

Overall this looks goot to me! Could you add an upgradestep from version 8->9 so that the lastcompiled-date gets updated in the registry? @thet I think then we could merge and release these improvements... what do you think?

@thet
Copy link
Member

thet commented Jul 23, 2019

I agree. Thanks for your work and caring about this project @djay
Can someone else take care of merging and releasing? I'm on vacation now.
@djay @petschki you both got maintainer access to the project on https://pypi.org/project/collective.collectionfilter/ , so make a release :)

@petschki petschki merged commit 040fdb5 into master Jul 23, 2019
@petschki petschki deleted the djay/restore_5.0.x branch July 23, 2019 08:04
@petschki
Copy link
Member

done!

@petschki petschki changed the title [WIP] restore 5.0.x compatibility Restore 5.0.x compatibility Jul 23, 2019
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