tibdex / autosquash

:package: GitHub Action to update PRs with outdated checks and squash and merge the ones matching all branch protections
MIT License
140 stars 24 forks source link

Autosquash is appending the PR's body #24

Closed MaicolBen closed 4 years ago

MaicolBen commented 4 years ago

I see it described in "The description of the pull request will become the message of the squashed commit."

Why is doing that? I don't see an option to disallow it.

MaicolBen commented 4 years ago

For anyone interested, you can remove it by removing this line commit_message: getSquashedCommitMessage({ body, coAuthors }),.

It seems that autorebase doesn't do that, so we should keep that behavior by default

tibdex commented 4 years ago

One of the goals of Autosquash is to be able to not care too much about the Git history of a pull request's head branch while still guaranteeing a clean history on its base branch e.g. master.

When doing code reviews with a CI in place, a pull request will often contains commits to fix formatting, linting, and tests, other to take into accounts the comments of reviewers, and maybe merge commits to stay in sync with the base branch. Most probably, you don't want the messages of these commits to appear in the description of the squashed commit created by Autosquash when merging the pull request.

The one thing that describes this squashed commit the best and that's easy to edit and format is actually the pull request's title and description. That's why I picked these to create the message of the squashed commit.

MaicolBen commented 4 years ago

a pull request will often contain commits to fix formatting, linting, and tests, other to take into accounts the comments of reviewers,

I totally agree!

the pull request's title and description

That might depend on the organization, in our case, we use the description for code review checklist

Maybe, the pull request's description is specific to us, but what about the co-authors? It puts myself as I put the secret token, so it uses that for merging. I think this should be configurable in the future.

Anyway, thank you for doing this tool, it's awesome!

tibdex commented 4 years ago

If you have a suggestion (that doesn't require configuration) to generate a better squashed commit message that wouldn't include all the unwanted intermediate commit messages, I would gladly consider changing the current behavior ;)

MaicolBen commented 4 years ago

Is it impossible to configure github wokflows (I'm not familiar with them)? If that's the case, you can close this. The github's default isn't the best but in my case the PR's body shouldn't be in the body. If we are waiting for more people to complain about this, we can come let this open.

Thank you!

tibdex commented 4 years ago

It puts myself as I put the secret token, so it uses that for merging

I have a private repo when we the secret token used by Autosquash is also a personal access token from a team member and so the merge commits are seen by GitHub as authored and committed by him. But he's not added as a co-author of all PRs thanks to this line:

https://github.com/tibdex/autosquash/blob/9151c7905017dfd2c4bd2bc3635553e8c61bacf7/src/autosquash.ts#L226-L227

Is it impossible to configure github wokflows

It's possible but I try to keep this action zero-config for simplicity's sake. But again, if you have a suggestion for what would be a better squashed commit message in general, I'm happy to change it.

MaicolBen commented 4 years ago

of all PRs thanks to this line

So that happened to me only 😞

if you have a suggestion

Kodiak/Bulldozer handle several options (github default, summarize commits, etc), but I get it you want to make it simple and keep the current behavior to what applies to you, keep it that way, ultimately, it has to work for you that you are maintaining it.

I'm ok to close this.

OPeyrusse commented 4 years ago

I have the same issue as @MaicolBen. Not to have something configurable, we could instead decide of a convention. Many organizations use markers to delimit a part ignored by system. Gitlab support email contains this:

- Please type your reply above this line -

so that users know where to type their answer and the system knows what it should pick in the message.

Because Github supports markdown, I suggest that everything before the first horizontal rule should be treated as commit message.

The rationale for choosing the first horizontal rule as opposed to the last one is that I don't see git commits messages with such an element, while you may want to have many sections in your PR checklist, separated with rules.

This seems quite easy to do in #getSquashedCommitMessage, since we have the body of the PR.

I'll prepare a PR for that, on which we can discuss.