search

mattermost / mattermost-plugin-github

GitHub plugin for Mattermost
Apache License 2.0
157 stars 148 forks source link

[GH-719] Pushed commits events: add option to show Author instead of committer #729

Closed cpatulea closed 7 months ago

cpatulea commented 8 months ago

Summary

Pushed commits events: add option to show Author instead of Committer.

Screenshot

image

What to test?

Before starting, some Mattermost channel should be subscribed to pushed commits notifications for a repo.

  1. Go to plugin settings and set "Pushed commits events: show Author instead of Committer:" to true.
  2. Create a git commit with committer != author. For example, "git am" a patch from someone else, or "git commit --author 'Test author test@example.com'" (example: https://github.com/FOULAB/eigma-test/commit/65ed77834f659a0498c5d02cc319e38bd12c50f3)
  3. Push commit to GitHub
  4. Observe the pushed commits notification.

Ticket Link

Fixes https://github.com/mattermost/mattermost-plugin-github/issues/719

mattermost-build commented 8 months ago

Hello @cpatulea,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

cpatulea commented 7 months ago

Hi @hanzei would you be able to approve for merge, or reassign to someone else if needed? Thanks in advance.

codecov-commenter commented 7 months ago

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (5aa2450) 15.79% compared to head (5e0a5f2) 15.81%. Report is 1 commits behind head on master.

:exclamation: Current head 5e0a5f2 differs from pull request most recent head fe1ed95. Consider uploading reports for the commit fe1ed95 to get more accurate results

Files Patch % Lines
server/plugin/template.go 44.44% 4 Missing and 1 partial :warning:
server/plugin/webhook.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #729 +/- ## ========================================== + Coverage 15.79% 15.81% +0.01% ========================================== Files 15 15 Lines 5767 5779 +12 ========================================== + Hits 911 914 +3 - Misses 4814 4822 +8 - Partials 42 43 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cpatulea commented 7 months ago

I would love to see a test for the changes before merging it.

Done, PTAL.

hanzei commented 7 months ago

@cpatulea I've left two nit pick, but the change LGTM. Please let me know if you want to address them.

cpatulea commented 7 months ago

@cpatulea I've left two nit pick, but the change LGTM. Please let me know if you want to address them.

Yep, I have addressed them.

cpatulea commented 7 months ago

server/plugin/template.go:112: File is not gofmt-ed with -s (gofmt)

Done.

cpatulea commented 7 months ago

Hey @AayushChaudhary0001 would you be able to take a look when you have a second?

AayushChaudhary0001 commented 7 months ago

@cpatulea Yes, I will review it mostly by this week.

mickmister commented 7 months ago

Thanks @cpatulea!