Adding .git-blame-ignore-revs for "bulk cosmetic changes" on Rails Guides

hachi8833 · February 24, 2023, 12:52pm

I’ve been wondering if I could perform bulk cosmetic changes on Rails Guides for integrity. Currently the inconsistency disturbs us when updating Rails Guides translations.

Of course I recognize that a policy that is stated in #13771: staggered cosmetic changes would disturb other developers especially when git blame. So I thought I had no chance to do that.

Recently git added a new feature: blame.ignoreRevsFile config options on 209f075.

Adding .git-blame-ignore-revs file to the project, and perform the following:

$ git config --global blame.ignoreRevsFile .git-blame-ignore-revs

Then we can add the cosmetic commits to .git-blame-ignore-revs to ignore them when git blame. GitHub already supports the feature on Web UI, assuming the filename .git-blame-ignore-revs.

I guess this feature may be a good tool for us to perform bulk cosmetic changes on Rails Guides to make the style consistent.

Considerations:

Each developer has to perform the git config above to activate them?
Whether the filename .git-blame-ignore-revs is ready for other repositories or tools such as GitLab or Git Lens
The bulk cosmetic commits should be squashed to avoid slowing down.
ruby_on_rails_guides_guidelines.md should also be updated to describe this.

Any ideas or mentions?

zzak · March 2, 2023, 10:39pm

I’m not sure, but it seems ignoring specific revisions would be per-client.

Do you have any examples of changes you’d like to make?

hachi8833 · March 4, 2023, 12:57am

Thank you for the reply!

The most changes I’d like to to make:

fragmented paragraphs by around 80 chars:

Seems most people don’t mind that (perhaps because this does not effect the html), but this is very bad for translation to other language.

fix missing language indications on code blocks, such as bash or html+erb
change Note. or Warn. to NOTE: or WARNING:
Use the separated link style (highlighted yellow) for external links

I recognize that the priority of such trivial and cosmetic changes is low. I wish I just had a chance to do that.

zzak · March 23, 2023, 9:03am

@hachi8833 Sorry for the delay, after a quick think. I want to get rubocop-md and markdownlint merged before making any major changes.

github.com/rails/rails

Rubocop markdown snippets

rails:main ← zzak:rubocop-md

opened 09:46AM - 30 Jan 23 UTC

zzak

+181 -155

Introduce [rubocop-md](https://github.com/rubocop/rubocop-md) for our markdown f…iles. This ensures any code snippets in the guides also follow the Rails coding conventions, without having to manually review every change that comes in. I've intentionally disabled `Style/StringLiterals` in the guides because that is a much larger change, and figured I would follow up with a single PR to do that after this gets merged. You can see some of the changes broken down further in zzak/rails#2 and zzak/rails#3.

The idea is that we can then enforce these decisions and not have to constantly monitor PRs that don’t adopt this style.

It seems really straight forward to implement custom markdown lint rules as well!

hachi8833 · March 28, 2023, 3:00am

Sounds nice! I agree with the idea that just leaving them to RuboCop is the best.

zzak · March 28, 2023, 8:04am

I’ve cleaned up and got this PR ready:

github.com/rails/rails

Introduce markdownlint for guides

rails:main ← zzak:mdl

opened 03:18AM - 27 Mar 23 UTC

zzak

+178 -92

This is a follow up to #47186, this time for all markdown content. [markdownl…int](https://github.com/markdownlint/markdownlint) is an excellent tool, and I've found it very useful for finding issues in the guides. Many of the rules are common style issues I'm correcting on PRs, so it will be nice to have that automated. We should also be able to use the same config with our editors, so that errors show up in real-time 🙏 and will update the contributing docs once this gets merged with how to debug and use mdl appropriately.

They provide many existing rules and it seems straight-forward to add our own custom rules, like for the ones you’re suggesting. There is already a rule for line-length, which I’ve set to 2000 but we can put it somewhere more reasonable, like 200. I’m not sure how you would detect fragmented paragraphs however, still some interesting stuff to work with that will hopefully save us and contributors time in code review.