collective / Products.PasswordStrength

Adds verification rules for user passwords in plone.
http://plone.org/products/passwordstrength
Other
5 stars 4 forks source link

Compatibility with 4.3, tests and installation fixes #5

Closed djay closed 9 years ago

sgeulette commented 10 years ago

@djay isn't it better to make a new master release with the last modifications of macagua and tagged as Plone < 4.1, and after that merge the branch and release...

gbastien commented 9 years ago

Hi @djay

could you please make the 2 releases or give me (and/or sgeulette) rights to do it on pypi?

The idea is a 0.3.2 with current master marked as for Plone < 4.1 and a 0.4 (probably?) with this PR merged?

Thank you!

gbastien commented 9 years ago

Hi @djay

when you have time ;-) But you could also give us some rights on pypi so we can do what I explained here above ;-)

Thank you!

Gauthier

djay commented 9 years ago

There is a blocking issue with the code that is preventing release https://github.com/collective/Products.PasswordStrength/issues/6. I'm also still not sure why additional monkey patches needed to be made.

sgeulette commented 9 years ago

@djay @gbastien I have pushed a solution for issue #6. For the other point: the added monkey patches are strictly needed for Plone 4.1 and 4.2. If you try to remove it and run tests, you will see what happens.

sgeulette commented 9 years ago

Hello @djay , Can we now make the two releases as explained ? We can do the job if we have rights on pypi. Thanks in advance

gbastien commented 9 years ago

Hi @djay, @thomasw, @regebro, @gotcha

could we please go on with this PR?

What we intend to do is move current master to a branch 0.3.x then merge the PR into the master and make a 0.4.0 release.

Is that alright? Code as presented with monkeypatches is necessary for Plone 4.1, 4.2 and 4.3 support together. Future release could drop support for 4.1 and 4.2 and we could remove these monkey patches.

Could anybody give pypi rights to sgeulette to do this?

Thank you!

Gauthier

thomasw commented 9 years ago

@gbastien I have absolutely nothing to do with this repo, so I'm assuming that pinging me was an accident. I did a bit of code review on it anyway while I had some down time after my lunch today, but I didn't make it through the entire pull.

djay commented 9 years ago

I just tried it on 4.3.2 and got it blanking the password input on a validation error.

djay commented 9 years ago

Why did you merge it if it doesn't work correctly?

sgeulette commented 9 years ago

Hi djay, I have merged before seeing your message. The tests ran correctly on the branch. I'm currently fixing the travis tests on master because .travis.yml now do coverage... Do you want I make a 0.3.2 release of the old code ?

gotcha commented 9 years ago

@djay Not sure I understand what you mean. I just tested standard Plone 4.3.5 (without PasswordStrength) and it does blank password input (imo for obvious security reasons). Does PasswordStrength used to try sthing else ?