-
-
Notifications
You must be signed in to change notification settings - Fork 176
integrate coredev.buildout (was PR #1670) #1671
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
Conversation
…d with a MyST extension we use), thus simplifying the diff review against `6.0`.
…'s not an explanation or reference. - Purge git and GitHub material, and refer to first-time contributors. - Modify pre-requisites
I could really use a review from anyone who cares about how to contribute to and develop in Plone core of just this one file (https://plone6--1671.org.readthedocs.build/contributing/core/index.html) in this commit: e2fb4f8 @jensens @petschki @ericof @MrTango @sneridagh @davisagli @thet This one file is the key to getting new For anyone who cares about releasing Plone, specifically @mauritsvanrees, I especially would like a review on https://plone6--1671.org.readthedocs.build/contributing/core/release.html More to come, but these two pages are super critical to the others. |
When it comes to "Edit packages" I always create a
and then run
so you do not have to edit any of the original files ( which might also get manipulated by mr.roboto ... eg. checkouts.cfg ) |
@thet also suggested something similar.
|
…cus on how to do things.
Revise process overview without the FWT, as it appears to be an ex-FWT, or possibly just pining for the fjords.
the changes I requested were done but I didn't have time to review the other parts of the PR yet
@davisagli I would like to merge this after the Volto Team meeting on Tuesday, July 2. Will that work for you? |
Sorry for the delay, I'll look into it ASAP. |
"keywords": "Plone, continuous integration, best practices" | ||
--- | ||
|
||
# Essential continuous integration practices |
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 whole page is outdated, it does not mention mr.roboto
, GHA nor plone/meta
which make almost all the rules described outdated.
Sorry that it took me forever to react on the call to help on this PR 🙇🏾
I can do the update, should I push it directly to this branch? 🤔
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.
@gforcada please create a PR against this branch for review. Do not push commits directly to this branch.
I'm not clear on which repositories plone/meta
applies. From what I know, many repositories use make
and not tox
, including Volto, plone.restapi, and Documentation. Until there is both agreement and implementation for a unified approach, I don't know whether plone/meta
should be declared as authoritative for all projects under the Plone GitHub organization.
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.
Ok, thanks I will do that (the branch).
As for plone/meta
or make
, well, it's literally all but volto related packages that already use plone/meta
. There are a few that are missing (like CMFPlone) that I did not had the time to apply plone/meta
to it.
But this documentation is meant for classic UI, or? 🤔 I meant I saw quite a few references that always branch to say "if you are doing volto then see ../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.
This part of the documentation is intended for anything that is not Volto, which includes Classic UI, core add-ons, and other core packages.
There is also an effort with cookiecutter and mxdev, which is intended to replace buildout, at which point another rewrite of docs will need to take place.
I don't know how that reconciles with plone/meta
. I just document what I understand.
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.
plone/meta
and mxdev
/cookiecutter
are orthogonal, at least I'm 100% sure with mxdev
, with cookiecutter
that depends on what's being done, of for which purpose is used 😓
I've seen so many cookieXX
repositories, that I'm no longer sure what one should use 🤷🏾
but if the goal is to replace zc.buildout
, then plone/meta
has nothing to do with that and should be fine to document.
Actually plone/meta
has quite some exhaustive documentation on its own 😄
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.
@gforcada even before your review and subsequent pull request, I had a lot of issues with this document. It never had a high-level review. I apologize for not bringing up my concerns earlier, but here they are now.
- It's not clear what is the purpose of this document.
- It's not clear who is the target audience.
- Is it a tutorial, a how-to guide, an explanation, or a reference? See https://diataxis.fr/ for details of what these categories mean.
- Much of the content is redundant to existing parts of the documentation, as well as within this pull request, including:
Once we have an idea of what we want to do, then we can rewrite this document to achieve that goal. I'd be happy to have a chat with you in real-time in Discord, or continue the conversation here.
For cookie*, the primary repo is https://github.com/plone/cookieplone. It uses templates found in https://github.com/plone/cookieplone-templates. I don't know whether that repo uses plone/meta
itself, but its templates do.
cookie* uses mxdev, as described in https://6.docs.plone.org/install/manage-add-ons-packages.html#manage-plone-backend-packages-with-mxdev.
Are these files plone/meta
's documentation?
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.
In reverse order 🙃
-
yes, that's the documentation for
plone/meta
-
oh nice, thanks for the cookie pointers, now it's a bit more clear to me, the (not to be answered and solved now) is how that works with mr.bob and bobtemplates.plone and all that ecosystem, or again, we have cookie* for volto and mr.bob for classic UI? 🤔
-
for this document, I would say that it's an explanation of how the CI system works, being the target audience newcomers.
With the links you provided (thanks!) I would say it could be linked from https://6.docs.plone.org/contributing/index.html#continuous-integration, then, we could simplify my PR to remove the explanation about PRs and be more focused only on the tooling involved.
How that sounds?
As for talking in discord, sure, this week, I do have time in European evenings, something like https://www.timeanddate.com/worldclock/converter.html?iso=20240708T170000&p1=37&p2=202 ?
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.
-
OK on meta docs.
-
I encourage you to try cookieplone for fun. If there is something missing, or is not clear how to do, then file an issue.
-
I think an explanation page would be good. I do not know all the parts of CI, what each part does, or how they al work together to fulfill specified tasks.
In that context, let's change the document title to "Continuous integration". If we keep "essential" or "practices", then we start telling the reader how to do things or what things they should do, which makes it a how-to guide, not an explanation.
Also if you use Mermaid for diagrams, you could even draw a picture of the workflow.
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.
@gforcada to get this PR merged, I will remove the CI stuff entirely. It's not correct and should not be published in its current state. The rest of it is good. We can add the CI stuff in another PR.
I'll work on pulling it out of this PR, and moving it back to the coredev directory to preserve the grammar and MyST work I did. I'll do that sometime this week.
Co-authored-by: Gil Forcada Codinachs <gil.gnome@gmail.com>
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.
LGTM!
Thanks everyone! Merging! @gforcada let's work on the CI page when you get the chance. |
Thanks for pushing this, Steve! |
Preview of pull request
https://plone6--1671.org.readthedocs.build/contributing/core/index.html
Description
This time without submodules
ToDo before merge
@jensens insert a Mermaid diagram in docs at https://github.com/plone/documentation/pull/1671/files#diff-36199f263c1ec44ec26fc136ea22e39344bf44acc065e03e0fe26bc263d2f9f2.See Insert a Mermaid diagram inconceptual-guides/package-dependencies.md
#1683coredev
.ToDo after merge
http://docs.plone.org/develop/coredev/docs/plips.html
to point to new docs.Closes
Products.CMFPlone
is broken Products.CMFPlone#3973References