Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Amend version policy docs #6541
base: main
Are you sure you want to change the base?
Amend version policy docs #6541
Changes from 9 commits
10459fd
1cfe19c
1a3ddef
7d0d06d
8dc2ab7
3bb6191
097dda6
23723b9
99292c6
3b84aca
427e449
50f1b39
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should say something more for
17.x.x
? For example, let's say I have a project with Plone 6.0 and Volto 17. What should I do? Should I upgrade both Plone to 6.1 and Volto to 18? If so, then we need to also link directly to each upgrade guide.Note that we can't use a label in an include file that is used in multiple places in Sphinx (duplicate label warning), so we have to write "(see above definition)".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not mandatory to upgrade. It can get some updates, if people still care for it, and the Volto Team approve them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneridagh Steve's right, if we're saying it's no longer supported, we need to give some advice on how to move forward.
It's possible to upgrade to Volto 18 (i.e. by generating a new cookieplone project and moving your custom code over) without updating the backend to Plone 6.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I edited my suggestion to adapt to the new "legacy" term.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second sentence in my suggestion below was omitted from the Upgrade Guide https://6.docs.plone.org/volto/upgrade-guide/index.html#cookieplone-is-now-the-recommended-project-and-add-on-generator-for-volto-18. Should we add it there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to contradict line 41.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm adding a "legacy" entry at the top of the document to clarify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't tell people to submit security issues or pull requests publicly. It should go through [email protected], unless I'm totally off about that. True story, I once got slapped for creating an issue stating that a Plone repo's primary branch lacked branch protection, even though it was stated publicly on the repo's home page.
Also I think we should tell people to create an issue to discuss a bug fix with the Volto Team before doing any work with a pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should link to https://6.docs.plone.org/install/create-project-cookieplone.html here, not the Cookieplone repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole sentence needs a rewrite. This syntax should work via Intersphinx for both here and
plone/documentation
.=>
For new projects, follow Install Plone with Cookieplone instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next line makes it sound like it's still supported for Volto 18, we need to change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it work in 18? I assumed that if something works, then that means it's supported.
Also if something works, it can also be deprecated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we started to update it for Volto 18 but then realized we needed to create Cookieplone instead, because the structure was too different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it doesn't work in 18, then why do we even have it in 18? https://github.com/plone/volto/tree/main/packages/generator-volto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"It does work" but it's true that maybe we should not advertise it, it could be confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version of @plone/volto-generator for Volto 18 was created quite early in the process of developing Volto 18, and it hasn't really gotten much attention as further changes have happened after we switched to recommending cookieplone. It doesn't surprise me that it's not working fully, and I don't think it's worth a lot of effort to fix it. We need to update trainings to use cookieplone or be clear that they are for an older version of Volto.
@wesleybl There's a new structure for Volto 18, and there's no longer a distinction between an app and an addon. (Similar to how in the backend a project and an addon are both just Python packages.) Use the
project
template in cookieplone if you want to generate both a backend and a frontend, or thefrontend_addon
template if you only want to generate a frontend (for use with an existing backend).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneridagh this came up in #6528 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davisagli do we also need to update these items in the upgrade guide? The first implies it's all good, especially when the second says it really ain't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davisagli I have some considerations about this. I think there should be an app template. However, I believe this is not the place to discuss it. I’ll create an issue in the cookieplone repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
@plone/volto-generator
doesn't work 100% with Volto 18, I also think it should be removed from the compatibility table.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awaiting response to my question in plone/Products.CMFPlone#3926 (comment) for Python 3.13 support. We can amend later, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amend it later. Python 3.13 support is close, but not released yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a couple of comments in the release checklists just now, so we don't forget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have released 6.0.14 and 6.1.0b2 with Python 3.13 support. I have added commit 50f1b39 to this PR to reflect this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always check your local build, or at least verify on RTD previews https://volto--6541.org.readthedocs.build/theming/custom-styling.html.
All includes must be relative to the current file, not the documentation root, else it will break in the main documentation. Please update the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.