-
-
Notifications
You must be signed in to change notification settings - Fork 88
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
Overhaul training "Mastering Plone 6 development" #852
base: main
Are you sure you want to change the base?
Conversation
ksuess
commented
Oct 7, 2024
•
edited
Loading
edited
- new chapter 'search results block variation'
- new chapter 'Search for additional fields'
- enhanced chapter testing (frontend acceptance tests with backend package installed)
- add-ons with cookieplone
- enhanced chapter intros and code hints
- and many more
✅ Deploy Preview for plone-training ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
…er with backend add-on from repo is clear and understandable
Hi @stevepiercy, do you have a hint for me why the search field on the top left does not show up? I am clueless, cause i just modified files in /docs, no configuration or sphinx templates at all. |
@ksuess computer in shop. No idea. |
I see it. Try a wider screen? CMD-K should also activate the search field. |
I confirm that when I narrow the browser window, the search input box in the left nav gets replaced by a magnifying glass icon in the top nav. |
The netlify preview does not show the search field on top left. I'll leave it at that, cause you are preparing Plone Sphinx theme for documentation and training and the search feature is still available via the button on top right, OK? |
Netlify can't be used for pull request previews because they don't support modern Python versions (3.8 is the latest, if I recall correctly). We need to migrate Training to RTD PR previews, as I did for Documentation, plone.restapi, plone.api, and volto. Until then, use local preview builds. I haven't hit this repo yet, as there usually is no activity until the week before PloneConf. |
See #853 for RTD PR previews. |
RTD PR previews will work for all new PRs. Example https://plone-training--858.org.readthedocs.build/en/858/contributing/setup-build.html#id1. |
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 only had time to review one page. It looks good. Mostly capitalization of proper nouns, but also update a few references. I'm sure it's OK to merge.
```` | ||
|
||
It's a good practice to write tests for the main requirements of a project. The **requirements are getting clearer and a path for the development is pointed**. | ||
|
||
This chapter is meant as a starting point for testing in Volto. | ||
|
||
<!-- | ||
|
||
% TODO Configure backend acceptance server with backend add-on 'training.votable' from repo. |
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.
Is there something in Plone 6 Docs that needs to be done? We're working on several things at the moment.
preview on readthedocs.org works really well. Thanks for the quick implementation, @stevepiercy ! |
@ksuess you're welcome! Also all new pull requests with changes to the docs will get a link inserted into the pull request description to preview the docs on RTD, and the email notification of a new PR will include that link. How would you like to do reviews of this PR? It's huge. If there's time, I could do a more thorough review of this PR. But instead of going through GitHub, I would check out your branch, edit, commit, and push. It would be much faster and easier for me to do replacements, update the Vale "allow" and "reject" lists, and other things. Or you could just merge it to get it done, then ask folks to run through it and submit feedback. Please let me know. Thank you! |
I think the complete text would benefit of a review, not just the changes here in this 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.
@ksuess merge now, separate PRs for each page as time allows sounds good to me. Thank you!