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

[docs] Advise contributors to check for truncated PR titles #68021

Merged
merged 1 commit into from
Oct 2, 2023

Conversation

asb
Copy link
Contributor

@asb asb commented Oct 2, 2023

GitHub will use the first line of the commit as the title for a single-commit PR, but truncates it at 72 characters https://github.com/orgs/community/discussions/12450. This truncation makes the PRs less readable if not manually undone, and even worse, the truncated form may survive through to commit if "Squash and rebase" is used in the GitHub web UI. From preparing LLVM Weekly, I've seen this a number of times and it really does make it more annoying to flick through commits.

I'm not sure if this is the best place for the guidance, or whether you get the same behaviour when creating a PR with gh, but I'm quite keen we give a warning of some sort about this behaviour.

GitHub will use the first line of the commit as the title for a
single-commit PR, but truncates it at 72 characters
<https://github.com/orgs/community/discussions/12450>. This truncation
makes the PRs less readable if not manually undone, and even worse, the
truncated form may survive through to commit if "Squash and rebase" is
used in the GitHub web UI. From preparing LLVM Weekly, I've seen this
a number of times and it really does make it more annoying to flick
through commits.

I'm not sure if this is the best place for the guidance, or whether you
get the same behaviour when creating a PR with `gh`, but I'm quite keen
we give a warning of some sort about this behaviour.
@asb asb requested review from tru, tstellar and joker-eph October 2, 2023 19:10
@joker-eph
Copy link
Collaborator

What about having our bots check for this pattern and advise the user to fix it?

This is also a problem of reviewers not flagging it by the way.

@asb
Copy link
Contributor Author

asb commented Oct 2, 2023

This is also a problem of reviewers not flagging it by the way.

That's a fair point - I think it's made a bit more difficult by the fact the relationship between the commit message and the PR is less clear than before. e.g. someone who lands their commit by updating the branch then merging it from the git CLI would never see this issue, even if they left GitHub's UI to mangle the commit title for their PR.

@tstellar
Copy link
Collaborator

tstellar commented Oct 2, 2023

What about having our bots check for this pattern and advise the user to fix it?

I don't think this would be hard to implement, but I'm fine with the documentation change until someone implements this.

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM

I agree this is a definite anti-feature in github, and warning about it for now is a good step.

@asb asb merged commit 3dda104 into llvm:main Oct 2, 2023
2 of 3 checks passed
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 this pull request may close these issues.

None yet

4 participants