plone / Products.PlonePAS

User and group implementation for the Plone CMS and Zope PluggableAuthService
6 stars 21 forks source link

Fix deprecated imports #51

Closed thet closed 4 years ago

mister-roboto commented 5 years ago

@thet thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

thet commented 4 years ago

@jenkins-plone-org please run jobs

thet commented 4 years ago

Done @mauritsvanrees

mauritsvanrees commented 4 years ago

Actually, the tests were passing on Jenkins before your last commit, and this last commit was only about Travis. So for me it is green.

The PR title has "WIP" in it though. Is that still accurate? And can you change the title to something more informative? :-) The first commit message seems good: "Plone 5.2 compatibility: fix deprecated imports".

thet commented 4 years ago

@mauritsvanrees I changed the title. I was also thinking of removing the travis config. This is already being tested by Jenkins, so I don't see a point in testing it on Travis too. Which repos should be tested by travis and which not? Addons?

There is some dysfunctional travis config in the "Services" section of this repositories settings, though... But obviously it's not working. I guess, I'll remove the travis config...?

mauritsvanrees commented 4 years ago

When a package is in Plone core, it is fine to be tested on Jenkins only. Exception may be a branch that is not used in core, like plone.app.contenttypes 1.1.x which is for Plone 4.3, when it was not yet in core. Another exception would be a package that may be useful outside of Plone, especially if the tests run much faster on Travis. Or a package that we want to test in other combinations (testing plone.protect 3.x with the csrf fixes on Plone 4.3 would be good).

Fine with me to remove the travis config here.

thet commented 4 years ago

Thanks for the explanation. That makes much sense.