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

If and how to PR a great deal of changes #1817

Open
QuineDot opened this issue Feb 17, 2024 · 3 comments
Open

If and how to PR a great deal of changes #1817

QuineDot opened this issue Feb 17, 2024 · 3 comments

Comments

@QuineDot
Copy link

I ended up reviewing most of this book, and have come to the conclusion that the book needs a non-trivial number of changes to bring it more up-to-date and to correct some inaccuracies.

You can read my notes here.

If you (the maintainers) wish, I can see about turning my notes into PRs. But as you can see, there are a lot of notes. My questions are

  1. Would this be productive -- can you entertain a large number of changes?
  2. If so, how best should the PRs be made -- per page, per chapter?
  3. Is it okay restructure sections/pages or are you trying to maintain the page names, etc?

Alternatively, feel free to make use of the notes without my further involvement.

@marioidival
Copy link
Member

Hi @QuineDot thank you! I just read your notes and I agree with everything from there!

About your questions, It's ok with that for all.

@simonsan
Copy link

simonsan commented Mar 18, 2024

Disclaimer: Not a maintainer, but talking from my experience with Rust Patterns book.

  • If so, how best should the PRs be made -- per page, per chapter?

Per chapter changes work the best IMO, like easy to review (without a lot of context switches) pieces of text for one topic.

  • Is it okay restructure sections/pages or are you trying to maintain the page names, etc?

If you do that, and rename a page, don't forget to set up a redirect so old links, e.g. in search engines, stay alive. Here is an example from Rust patterns book.toml. Page names are determined by their markdown name.

[output.html.redirect]
# Redirects in the form of "old-path" = "new-path", where the new path
# is relative to the old path.
"functional/lenses.html" = "optics.html"

If it happens that there will be a lot of PRs, I would be able to also help to review them, in case they become overwhelming @marioidival 🤗

@simonsan
Copy link

simonsan commented Mar 18, 2024

Another important thing, actually. We use dprint in Rust patterns. I think it makes a lot of things easier, because the diffs are essentially always easy to read. I think there is also some setting, that formats the code blocks, I need to look into it. This would essentially make it much easier to merge the above changes. And in general keep the maintenance effort low, because there is one style to the code base.

EDIT: Formatted also the code blocks.
So I opened a PR to let you have a look how it looks for this codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants