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

Dx container site root #85

Merged
merged 43 commits into from
Aug 20, 2021
Merged

Dx container site root #85

merged 43 commits into from
Aug 20, 2021

Conversation

jaroel
Copy link
Member

@jaroel jaroel commented Jun 22, 2018

don't merge

@jensens
Copy link
Member

jensens commented Jun 23, 2018

@jaroel please dont use WIP in the title, but the labels provided. Using the labels I and others involved in PR management are able to filter the PRs and issues. Thanks!

@jaroel jaroel changed the title WIP Dxcontainer siteroot Dx container site root Jun 23, 2018
@jaroel jaroel force-pushed the dxcontainer-siteroot branch from 49f3890 to 9e036ea Compare July 30, 2018 19:56
CHANGES.rst Outdated Show resolved Hide resolved
Copy link
Member

@mauritsvanrees mauritsvanrees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I guess with a minor change (see inline comment) this might be mergeable already, even when the Plone Site is not yet dexterity.
I am going through the PRs to see which might be merged already, making the dexterity site root PLIP a bit smaller.

This would need yet another merge of master into this branch, or a rebase and force push, plus tested on its own in Jenkins.

@@ -326,7 +329,12 @@ def portalTypeToSchemaName(portal_type, schema=u"", prefix=None, suffix=None):
"""Return a canonical interface name for a generated schema interface.
"""
if prefix is None:
prefix = '/'.join(getUtility(ISiteRoot).getPhysicalPath())[1:]
if portal_type == 'Plone Site':
fti = getUtility(IDexterityFTI, name=portal_type)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use queryUtility here, and catch the case where it is None, then this PR could probably be merged already before all the other dexterity site root PRs. It is a guess.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

…aName.

That makes this code usable when Plone Site is not (yet) a dexterity item.
I doubt that this code path is ever reached when the Plone Site FTI is not dexterity, but let's be careful.
@mauritsvanrees
Copy link
Member

@jenkins-plone-org please run jobs

@mauritsvanrees
Copy link
Member

Jenkins is green on 5.2 and 6.0 without needing the other site root PRs. For me it is fine to merge.

@jensens jensens merged commit f0ca303 into master Aug 20, 2021
@jensens jensens deleted the dxcontainer-siteroot branch August 20, 2021 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants