volto icon indicating copy to clipboard operation
volto copied to clipboard

Many Volto documentation corrections

Open silviubogan opened this issue 7 months ago • 16 comments


Closes #7038


📚 Documentation preview 📚: https://volto--7115.org.readthedocs.build/

silviubogan avatar May 26 '25 12:05 silviubogan

@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 avatar May 29 '25 08:05 stevepiercy

@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 avatar May 30 '25 07:05 silviubogan

@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.

stevepiercy avatar May 30 '25 07:05 stevepiercy

@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.

@stevepiercy

  1. I am not used to rebases. I don't want to make big mistakes.
  2. 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?

silviubogan avatar May 30 '25 08:05 silviubogan

Your branch is named docs-seven-many-corrections, not docs-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 avatar May 30 '25 08:05 stevepiercy

Your branch is named docs-seven-many-corrections, not docs-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

  1. main
  2. Volto

When I started working on the related issue I did not even know what Seven is.

silviubogan avatar May 30 '25 08:05 silviubogan

OK, thanks for clarifying. I'll ignore the branch name, and start a review.

stevepiercy avatar May 30 '25 08:05 stevepiercy

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 avatar May 30 '25 08:05 stevepiercy

@stevepiercy Please stop temporarily the review, I still have some commits locally, but maybe I have to rebase them.

UPDATE: done.

silviubogan avatar May 30 '25 08:05 silviubogan

@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.

stevepiercy avatar May 30 '25 09:05 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.

stevepiercy avatar May 30 '25 10:05 stevepiercy

@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.

silviubogan avatar May 30 '25 12:05 silviubogan

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 avatar May 30 '25 12:05 stevepiercy

@stevepiercy

Please go ahead with the "Add to Batch" button for things you agree with.

I batch 105 code pieces and get an error:

image

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:

silviubogan avatar May 30 '25 14:05 silviubogan

I got the same error message. I'm going to try some git trickery in PyCharm to make this PR manageable.

stevepiercy avatar May 31 '25 20:05 stevepiercy

@plone/volto-team please review the items I've tagged for you to review in the unresolved suggestions.

stevepiercy avatar Jun 01 '25 11:06 stevepiercy

@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.

sneridagh avatar Jun 23 '25 16:06 sneridagh

@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.

stevepiercy avatar Jun 25 '25 09:06 stevepiercy

@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!

stevepiercy avatar Dec 04 '25 01:12 stevepiercy