cogs.util.github: Show merged PRs for multi-commit pushes by prgmitchell · Pull Request #18 · obsproject/obs-bot · GitHub
Skip to content

cogs.util.github: Show merged PRs for multi-commit pushes - #18

Open
prgmitchell wants to merge 1 commit into
obsproject:masterfrom
prgmitchell:showMergedPRs
Open

prgmitchell wants to merge 1 commit into
obsproject:masterfrom
prgmitchell:showMergedPRs

Conversation

@prgmitchell

Copy link
Copy Markdown
Member

Description

Show the matching merged PR for pushes containing multiple commits.

Motivation and Context

When multi-commit PRs get merged it makes more sense to see the PR itself rather than the just the last commit.

How Has This Been Tested?

It really hasn't, but seems good to me.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • I have read the contributing document.
  • My code has been run through clang-format.
  • My code follows the project's style guidelines
  • My code is not on the master branch.
  • My code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Comment thread obsbot/cogs/public/utils/github.py Outdated
Comment thread obsbot/cogs/public/utils/github.py Outdated
@Warchamp7

Copy link
Copy Markdown
Member

Show the matching merged PR for pushes containing multiple commits.
@prgmitchell

Copy link
Copy Markdown
Member Author

@Warchamp7 although I wanted to say just merge as-is, I agree splitting makes sense and know if it isn't done now it probably won't ever be. PR updated, let me know what you think.

@Warchamp7 Warchamp7 left a comment

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.

Non blocking review testing. Ignore me.

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.

I'm not a fan of doing an early return here instead of filling out embed_commits but it's fine.

I'm also only submitting this review to test something so you can ignore it entirely.

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.

2 participants