-
Notifications
You must be signed in to change notification settings - Fork 28
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
@openedx-community-bot
should merge with original commit message(s)
#134
Comments
You mean strange as potentially outdated or something else? Based on the description I was expecting a bug in the bot or a compromise. |
Sorry, I should have been more specific. What I find strange is that it does a squash+merge, throws out the existing commit message(s), and uses the PR description as the commit message instead. This leads to a very awkwardly long commit message, often including noise from the PR template (like the one linked). Since we are already linting individual commit messages, I think we should instead just do a rebase+merge or a standard merge, which would preserve the author's original commit messages. |
Ah, I clicked on the second link and was lost. Thanks. |
@openedx-community-bot merge
generates long, strange commit messages@openedx-community-bot
should use original commit message(s), not PR description
@openedx-community-bot
should use original commit message(s), not PR description@openedx-community-bot
should merge with original commit message(s)
Np. I rewrote the issue title and description to be more self-evident. |
@openedx-community-bot merge
was used to merge this PR. The bot use the "Squash & Merge" strategy, dropped Maria's original commit message, and replaced it with the PR description (which included noise from the PR template): openedx/edx-platform@a11ad35I think it would be better to do either a "Rebase & Merge" or a "Merge with Merge Commit", which would preserve the author's original commit messages and avoid copying in noise from the PR description. We currently lint commit messages, so it would make sense to use them.
The text was updated successfully, but these errors were encountered: