Skip to content

[improve][misc] Enable rebase and merge to handle one PR of multiple semantic commits#20325

Merged
tisonkun merged 1 commit intomasterfrom
tisonkun-patch-1
May 15, 2023
Merged

[improve][misc] Enable rebase and merge to handle one PR of multiple semantic commits#20325
tisonkun merged 1 commit intomasterfrom
tisonkun-patch-1

Conversation

@tisonkun
Copy link
Copy Markdown
Member

For example #20321.

Most of the time we use squash and merge, but rebase and merge help sometimes to keep commits separate while reviewing them at once.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@tisonkun tisonkun self-assigned this May 15, 2023
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label May 15, 2023
@tisonkun tisonkun merged commit 21afdfd into master May 15, 2023
@tisonkun tisonkun deleted the tisonkun-patch-1 branch May 15, 2023 12:34
Comment thread .asf.yaml
merge: false
# disable rebase button:
rebase: false
rebase: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might help to provide committers with documentation on when to use which button. We haven't had the option previously, so this might confuse some. I hope we don't get any committers arguing for a force push update to master to change the history because they hit the wrong button.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK. Let me start a mail thread first.

For documentation, I don't think committers read comments in .asf.yaml so probably a page under https://pulsar.apache.org/contribute/develop-coding-conventions/ said "Reviewing Pull Request" or "Merging Pull Request".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @tisonkun. I agree with putting it in that section. A committer guide would be valuable--I don't think we have one though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants