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.

Sign up for GitHub

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

Jump to bottom

Config with default template #262

Merged
merged 13 commits into from
Oct 19, 2023
Merged

Config with default template #262

merged 13 commits into from
Oct 19, 2023

Conversation

gforcada
Copy link
Sponsor Member

@gforcada gforcada commented Mar 9, 2023

It uses plone/meta#59 to configure the distribution.

@mister-roboto
Copy link

@gforcada thanks for creating this Pull Request and helping to improve Plone!

TL;DR: Finish pushing changes, pass all other checks, then paste a comment:

@jenkins-plone-org please run jobs

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass, but it takes 30-60 min. Other CI checks are usually much faster and the Plone Jenkins resources are limited, so when done pushing changes and all other checks pass either start all Jenkins PR jobs yourself, or simply add the comment above in this PR to start all the jobs automatically.

Happy hacking!

@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch from 91d0b81 to aa1a7d5 Compare March 9, 2023 22:40
mauritsvanrees
mauritsvanrees reviewed Mar 10, 2023
View reviewed changes
Copy link
Sponsor 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, but two small comments could be addressed.

tox.ini Outdated Show resolved Hide resolved
plone/app/content/browser/contents/templates/properties.pt Outdated Show resolved Hide resolved
@gforcada gforcada mentioned this pull request Mar 10, 2023
Merged
@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch 2 times, most recently from 60dec60 to c98e44b Compare March 16, 2023 08:32
@jensens
Copy link
Sponsor Member

jensens commented Mar 27, 2023

pre-commit.ci autofix

@jensens
Copy link
Sponsor Member

jensens commented Mar 28, 2023

@jenkins-plone-org please run jobs

This was referenced Apr 1, 2023
Closed
Closed
@gforcada gforcada marked this pull request as draft April 9, 2023 08:02
@jensens
Copy link
Sponsor Member

jensens commented Apr 11, 2023

@jenkins-plone-org please run jobs

@jensens jensens marked this pull request as ready for review April 11, 2023 16:31
@jensens
Copy link
Sponsor Member

jensens commented Apr 11, 2023

Some transitive dependencies are not detected. I suspect a bug in z3c.dependencychecker, because it works for some dependencies very well. Here packages Missing and Products.GenericSetup are not found, even if declared transitive.

@gforcada gforcada mentioned this pull request May 18, 2023
Merged
@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch 2 times, most recently from f42ed82 to 41a185c Compare June 18, 2023 07:49
@gforcada
Copy link
Sponsor Member Author

@mauritsvanrees now the problem with the <% markup is on i18ndude 🤦🏾

Should we actually refactor that to be proper HTML with the expected chameleon syntax? 😅

@mauritsvanrees
Copy link
Sponsor Member

@mauritsvanrees now the problem with the <% markup is on i18ndude 🤦🏾

Should we actually refactor that to be proper HTML with the expected chameleon syntax? 😅

I still don't quite understand what language (html? javascript? chameleon?) this even is.
If we can get this to work with normal html or javascript, that would be good.

@jensens
Copy link
Sponsor Member

jensens commented Jun 20, 2023

I still don't quite understand what language (html? javascript? chameleon?) this even is.
If we can get this to work with normal html or javascript, that would be good.

I think if i get this right this is a Zope page-template, where the rendered output is used again as a template in Javascript

@jensens
Copy link
Sponsor Member

jensens commented Oct 5, 2023

@gforcada can you guide me how to update this PR with latest Meta?

@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch 2 times, most recently from c462f2e to 1468d58 Compare October 14, 2023 18:04
@gforcada gforcada mentioned this pull request Oct 14, 2023
Closed
5 tasks
@gforcada
Copy link
Sponsor Member Author

We have a new circular dependency detected 😄 see plone/Products.CMFPlone#3858

@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch from 1468d58 to 33fc4eb Compare October 15, 2023 16:07
@gforcada
Copy link
Sponsor Member Author

I cheated a bit here 🤭 I remove the circular dependency check on GHA as that will fail until plone/Products.CMFPlone#3858 is fixed.

But meanwhile all those PRs get sorted out, we can benefit from adding plone/meta to this repository 🥳

@gforcada
Copy link
Sponsor Member Author

@jenkins-plone-org please run jobs

@mauritsvanrees
Copy link
Sponsor Member

All three Jenkins jobs fail with the same robot error, so it seems like a real error:
"Page should have contained element 'css=.breach-container .breach-item' but did not."

mauritsvanrees
mauritsvanrees approved these changes Oct 16, 2023
View reviewed changes
Copy link
Sponsor 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, except for the Jenkins test failures.

@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch from 33fc4eb to facb61c Compare October 17, 2023 20:14
@gforcada
Copy link
Sponsor Member Author

@jenkins-plone-org please run jobs

1 similar comment
@gforcada
Copy link
Sponsor Member Author

@jenkins-plone-org please run jobs

@gforcada
Copy link
Sponsor Member Author

zpretty is messing with some templates... but there are plenty of them 😅 I'm starting a sort of binary search on them 🍀

@gforcada gforcada mentioned this pull request Oct 18, 2023
Open
10 tasks
@gforcada gforcada force-pushed the config-with-default-template-7150e786 branch from 92758e9 to 756ade3 Compare October 18, 2023 09:30
@gforcada
Copy link
Sponsor Member Author

@jenkins-plone-org please run jobs

@gforcada gforcada merged commit 11bb056 into master Oct 19, 2023
12 checks passed
@gforcada gforcada deleted the config-with-default-template-7150e786 branch October 19, 2023 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jensens jensens jensens left review comments

@ale-rt ale-rt ale-rt left review comments

@mauritsvanrees mauritsvanrees mauritsvanrees approved these changes

Assignees
No one assigned
Labels
22 status: in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gforcada @mister-roboto @jensens @mauritsvanrees @ale-rt