Many Volto documentation corrections
- [x] I signed and returned the Plone Contributor Agreement, and received and accepted an invitation to join a team in the Plone GitHub organization.
- [x] I verified there aren't other open pull requests for the same change.
- [x] I followed the guidelines in Contributing to Volto.
- [x] I succesfully ran code linting checks on my changes locally.
- [x] I succesfully ran unit tests on my changes locally.
- [x] I succesfully ran acceptance tests on my changes locally.
- [x] If needed, I added new tests for my changes.
- [x] If needed, I added documentation for my changes, either in the Storybook or narrative documentation.
- [x] I included a change log entry in my commits.
Closes #7038
📚 Documentation preview 📚: https://volto--7115.org.readthedocs.build/
@avoinea I'm waiting until the PR is out of draft status. @silviubogan please let me know when it's ready for a review. Thank you!
@stevepiercy Here the assignment statements
// Make Vitest globals available throughout the test suite
global.describe = describe;
global.it = it;
global.expect = expect;
global.vi = vi;
are not required to use e.g. it in a new test file in Vitest or are they? I written a simple test and it works but I don't know where to find the setupTests.js file.
In rest, you can review this PR. Thanks.
@silviubogan Seven development is being done on the seven branch, not main. Please change the base of your PR. Once you do that, then I can review.
As far as your technical question, I have no clue. Someone more familiar with Seven will need to answer.
@silviubogan Seven development is being done on the
sevenbranch, notmain. Please change the base of your PR. Once you do that, then I can review.
@stevepiercy
- I am not used to rebases. I don't want to make big mistakes.
- The Volto docs and the Seven docs seem very different: https://volto.readthedocs.io/seven/
So I would suggest to accept the changes in the main branch. Then I would start studying and working on the Seven docs. I am sorry if it is hard to follow such a big PR. What is your opinion?
Your branch is named docs-seven-many-corrections, not docs-volto-many-corrections. You've confused me. Please clarify:
- Did you create this PR off of the branch
mainorseven? - Are your changes in this PR related to Volto or Seven?
Your branch is named
docs-seven-many-corrections, notdocs-volto-many-corrections. You've confused me. Please clarify:1. Did you create this PR off of the branch `main` or `seven`? 2. Are your changes in this PR related to Volto or Seven?
@stevepiercy
-
main - Volto
When I started working on the related issue I did not even know what Seven is.
OK, thanks for clarifying. I'll ignore the branch name, and start a review.
I assumed it was OK to change this PR's status from Draft to Ready to Review, so I clicked that button.
We can ignore the failing linkcheck for now. I opened a PR at https://github.com/collective/awesome-volto/pull/35 and notified @avoinea about the failing site https://climate-energy.eea.europa.eu not responding.
@stevepiercy Please stop temporarily the review, I still have some commits locally, but maybe I have to rebase them.
UPDATE: done.
@silviubogan let's freeze any more contributions to this PR. It is insanely huge for what is mostly trivial whitespace reformatting of docs and code. For such changes, they are best done in a separate PR that does only that and nothing else, otherwise material changes get lost in the noise. Additionally when there are dozens of whitespace changes, then you should probably create separate PRs, one per page, that can be more easily managed.
@silviubogan please let me know if you do not see the "Add to Batch" button. We don't want to spam everyone with 200 notifications.
@stevepiercy
@silviubogan please let me know if you do not see the "Add to Batch" button. We don't want to spam everyone with 200 notifications.
I see the Add to Batch button. Can I start creating the batch and create the commit, or do you wish to answer to my questions posted in the code here on GitHub? I've posted some questions in the Files Changed tab of the PR.
I agree with this, I will go again through all the Files Changed tab.
How can I be sure I see the entire discussions before going through all the files? I see I did not receive all the discussion linked above through email.
Please go ahead with the "Add to Batch" button for things you agree with.
We can work through the remaining unresolved suggestions and comments later. It's past my bedtime, and I think the Volto Team members mostly don't work on the weekend, like sane people.
@stevepiercy
Please go ahead with the "Add to Batch" button for things you agree with.
I batch 105 code pieces and get an error:
I tried in both Firefox and Chrome. I think it is a bug in GitHub. Next Monday it would be cool if you could try this on your own. Basically, I batched all the changes that were yours except those where a Volto Team technical request was required, or where me or you had commented.
like sane people.
Nice advice. :+1: Have a great weekend! :smiley:
I got the same error message. I'm going to try some git trickery in PyCharm to make this PR manageable.
@plone/volto-team please review the items I've tagged for you to review in the unresolved suggestions.
@stevepiercy @silviubogan I have a full review in the middle, I will continue adding things today.
@silviubogan However, this is a bit hellish, it would have been better to have this sliced in bits so it is more digest-able.
@silviubogan I spoke with @sneridagh during the sprint today. Although we've already spent a lot of time on it (I've personally spent over 10 hours on it), we decided that this PR is unmanageable and unmergeable.
We need to keep white space changes separate from substantial changes. Substantial changes to narrative text for English grammar, spelling, and syntax as well as MyST syntax should also be separate from changes to the structure, flow, and code samples. All that multiplied by changes across 32 files is too much. We experienced this through GitHub errors and browser crashes, not to mention the difficulty of tracking dozens of suggestions.
To move this forward, and only if you are willing to take it on, we would like you to create focused PRs on only one type of change: whitespace only changes; substantial English grammar, spelling, and syntax and MyST changes; and other substantial changes to structure, flow, and code samples. Alternatively, one file per PR would be possible to properly review. @silviubogan please let us know. Thank you!
For @plone/volto-team, when we see a new contributor, we should point them in the right direction as soon as possible. That didn't happen here for this PR until a lot of effort was put into it. I don't know if it was because we have very few people reviewing PRs in Volto, or if its docs in particular.
@silviubogan I'm closing this PR as abandoned and unmergeable. Please open new PRs as I suggested in https://github.com/plone/volto/pull/7115#issuecomment-3003991643 to move this forward. Thank you!