Skip to content
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

Rewrite help page section. #2821

Merged
merged 6 commits into from
Dec 24, 2024
Merged

Conversation

Rowlando13
Copy link
Collaborator

@Rowlando13 Rowlando13 commented Dec 15, 2024

Rewrite help page generation section, formerly documenting script.

Copy link
Collaborator

@AndreasBackx AndreasBackx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty hard to review because of the following:

  1. Git is not aware of the rename of documentation.rst -> help-pages.rst. This makes it hard to understand what has changed because it's all "new". See https://stackoverflow.com/questions/433111/how-to-make-git-mark-a-deleted-and-a-new-file-as-a-file-move.
  2. The sections have been re-ordered.

Could we do the following to make reviewing easier:

  1. Keep the order of the sections.
  2. Tell git of the rename.

Then a reviewer will be able to see the changes under each section.

If you still want to re-order sections, could that be done in a separate PR?

@AndreasBackx
Copy link
Collaborator

Could you also rebase this on main?

@Rowlando13
Copy link
Collaborator Author

Can do both.

@Rowlando13
Copy link
Collaborator Author

Do you want all docs based on main or just this branch?

@Rowlando13 Rowlando13 changed the base branch from stable to main December 23, 2024 02:19
@Rowlando13
Copy link
Collaborator Author

@AndreasBackx Done.

@AndreasBackx
Copy link
Collaborator

Do you want all docs based on main or just this branch?

All docs, let's focus on just main for now.

@AndreasBackx Done.

Thanks, this is much easier to review now.

Copy link
Collaborator

@AndreasBackx AndreasBackx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll approve to get this out the door. Though please make the non-nit changes. I definitely like that this is easier to read and skim, less fluff.

@Rowlando13
Copy link
Collaborator Author

I will make the changes. I don't think you have to worry about approving. I was able to merge without the review being fully approved on the last one.

@Rowlando13 Rowlando13 merged commit 8a47580 into pallets:main Dec 24, 2024
2 checks passed
@Rowlando13 Rowlando13 deleted the rewrite_help_pages_2 branch December 24, 2024 07:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants