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

Missing ReviewingGuidelines #1938

Closed
hickford opened this issue Jan 11, 2025 · 8 comments · Fixed by #1965
Closed

Missing ReviewingGuidelines #1938

hickford opened this issue Jan 11, 2025 · 8 comments · Fixed by #1965

Comments

@hickford
Copy link
Contributor

hickford commented Jan 11, 2025

Please publish https://github.com/git/git/blob/master/Documentation/ReviewingGuidelines.txt to https://git-scm.com/docs/ReviewingGuidelines

https://git-scm.com/docs/SubmittingPatches works as expected

@dscho
Copy link
Member

dscho commented Jan 12, 2025

https://git-scm.com/docs/SubmittingPatches works as expected

This works because of ea0e16d, which gave rise to do the same for MyFirstContribution and then also for MyFirstObjectWalk.

I think those three commits were mistakes. Why? Have a look at https://git-scm.com/docs/:

Image

Who is the audience? Clearly it is Git users. The vast vast vast majority of who will never even think of contributing to Git (and given the bar of entry, I think that that's a good thing). There is not a single good category where SubmittingPatches or ReviewingGuidelines would fit because those are not user-facing.

Therefore I would much rather change git-scm.com to redirect SubmittingPatches, MyFirstContribution and MyFirstObjectWalk (and ReviewingGuidelines and even `CodingGuidelines) to the git/git repository than to continue the bad practice of mixing documentation on git-scm.com that is clearly useful to Git users with documentation that is not.

@hickford
Copy link
Contributor Author

Therefore I would much rather change git-scm.com to redirect SubmittingPatches, MyFirstContribution and MyFirstObjectWalk (and ReviewingGuidelines #1939) to the git/git repository than to continue the bad practice of mixing documentation on git-scm.com that is clearly useful to Git users with documentation that is not.

I like that idea too

@dscho
Copy link
Member

dscho commented Jan 12, 2025

Therefore I would much rather change git-scm.com to redirect SubmittingPatches, MyFirstContribution and MyFirstObjectWalk (and ReviewingGuidelines #1939) to the git/git repository than to continue the bad practice of mixing documentation on git-scm.com that is clearly useful to Git users with documentation that is not.

I like that idea too

Good. To do that, you'll need to delete these lines and hard-code the pages to redirect by adding them to check_paths, like so:

# These are not user-facing docs, but concern the Git project specifically
check_paths.merge([
  "CodingGuidelines",
  "MyFirstContribution",
  "MyFirstObjectWalk",
  "ReviewingGuidelines",
  "SubmittingPatches"
])

How about giving it a try?

@dscho
Copy link
Member

dscho commented Jan 16, 2025

Hrm. Today I found myself guilty of wanting to link to https://git-scm.com/docs/SubmittingPatches#sign-off. The reason is that this is just such a short & sweet link that, thanks to the anchor, is also quite robust (the alternative would be https://github.com/git/git/blob/v2.48.1/Documentation/SubmittingPatches#L352-L408 and that line range changes all the time). Speaking of "changes all the time", it is also quite nice to be able to see just how much it changed over time:

Image

So I'll walk back on my earlier assessment. It is nice to have these Git project-specific pages on git-scm.com (because the Git project itself won't ever maintain a nice project page).

Maybe we should add a header (and adjust the translations drop-down) to clarify that these pages contain intentionally untranslated Git project-specific information?

Or maybe I am just overthinking this too much and nobody cares?

@hickford
Copy link
Contributor Author

hickford commented Jan 16, 2025

I think the best solution is to rename AsciiDoc documentation files upstream. GitHub renders *.adoc automatically, so you and I could then simply link to https://github.com/git/git/blob/master/Documentation/ReviewingGuidelines.adoc

https://lore.kernel.org/git/xmqqv7ui72e4.fsf@gitster.g/

@dscho
Copy link
Member

dscho commented Jan 17, 2025

The price, of course, would be to have formerly-working links and expectations being broken ;-) It's funny how selective that aim for backwards-compatibility seems to be in some people.

@To1ne
Copy link
Contributor

To1ne commented Feb 28, 2025

Today I found myself guilty of wanting to link to https://git-scm.com/docs/SubmittingPatches#sign-off.

@dscho LOL! 🤣

The reason is that this is just such a short & sweet link that, thanks to the anchor, is also quite robust (the alternative would be https://github.com/git/git/blob/v2.48.1/Documentation/SubmittingPatches#L352-L408 and that line range changes all the time). Speaking of "changes all the time", it is also quite nice to be able to see just how much it changed over time

True, various benefits. Also includes would work (although I doubt we need them at the moment).

Maybe we should add a header (and adjust the translations drop-down) to clarify that these pages contain intentionally untranslated Git project-specific information?

Sounds reasonable to me.

dscho added a commit to dscho/git-scm.com that referenced this issue Mar 1, 2025
Now that they are clearly labeled as such, let's augment the list of
"manual pages" that concern themselves with information only relevant
for contributors to the Git project itself.

This closes git#1938,
git#1939 and
git#1961.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git-scm.com that referenced this issue Mar 1, 2025
Now that they are clearly labeled as such, let's augment the list of
"manual pages" that concern themselves with information only relevant
for contributors to the Git project itself.

This closes git#1938,
git#1939 and
git#1961.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Mar 1, 2025

#1965

@dscho dscho closed this as completed in 1deace4 Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants