Use the Iterate permissions instead of CMF core permissions #68
Conversation
This comment has been minimized.
This comment has been minimized.
1 similar comment
|
@rpatterson 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: With this simple comment all the jobs will be started automatically. Happy hacking! |
|
@jenkins-plone-org please run jobs |
|
I can't imagine the Jenkins failures are related to these changes. |
|
@jenkins-plone-org please run jobs |
|
None of the failures are related to my changes AFAICT. |
|
@jenkins-plone-org please run jobs |
| @@ -29,7 +29,7 @@ | |||
| from plone.memoize.view import memoize | |||
| from Products.Five.browser import BrowserView | |||
|
|
|||
| import Products.CMFCore.permissions | |||
| from .. import permissions | |||
There was a problem hiding this comment.
Does this work as you expect in Python 3?
There was a problem hiding this comment.
Why wouldn't it, @Rotonen?
There was a problem hiding this comment.
Is fine, but it way better to just use from plone.app.iterate import permissions is not that much typing and you can keep imports sanely
There was a problem hiding this comment.
I disagree that absolute imports are way better. Relative imports do save some minor refactoring time when package names or structure changes, and if you're typing your imports out they do save some time though you should not be typing your imports out, but the far greater value is when reading code. They make it very clear which dependencies of the code are internal to the package and external to the package even when just skimming the code. This is especially the case for a large ecosystem of packages such as Plone where a developer may be working on 3-6 plone.app.* packages at the same time and it adds cognitive burden to require the developer to remember which plone.app.* package they're reading ATM in order to determine which code dependencies are internal. The core language and the community that develops it, provide this feature for well considered reasons.
|
@jenkins-plone-org please run jobs |
|
Hence there was no activity for a long time, I propose to close this PR within next two weeks. If you do not feel ok with this, please speak up and provide us a rough schedule. |
I still believe this is more correct and serves good use cases as described. But I probably don't have the time to shepherd this through ATM. |
Use the Iterate permissions instead of CMF core permissions to support customizing the policy.
Related to #59 and #67