Skip to content

More diffs in notification emails #24

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

Closed
warsaw opened this issue Feb 11, 2017 · 7 comments
Closed

More diffs in notification emails #24

warsaw opened this issue Feb 11, 2017 · 7 comments
Labels

Comments

@warsaw
Copy link
Member

warsaw commented Feb 11, 2017

If it's possible to support in GH, I would like to see diffs (unified) in PRs and commit emails. While it increases the size of the emails, I think it would be a huge productivity increase. Without the diffs, I have to stop and click on a link in my email, then go to my browser, just to see if the change is anything I can or want to comment on.

If the diffs were in my email notifications, I could do a quick review immediately. By scanning the email I could:

  • Decide that I don't care about this change, delete it and move on
  • Make a quick determination that the change looks good and do a quick +1 turnaround
  • See some changes that I have questions or concerns about but defer my response to when I have sufficient time
  • Have a quick comment about a change which I could send in a quick reply

I only want to go to the GH web ui when I really need to interact with the PR or commit, and that should be a minority of changes to Python.

@berkerpeksag
Copy link
Member

+1. I was going to open an issue for this :) Unfortunately it's not possible to include diffs in the email integration of GitHub. I can write a webhook to include diffs if we all want to see diffs in python-checkins emails.

@warsaw
Copy link
Member Author

warsaw commented Feb 11, 2017 via email

@berkerpeksag
Copy link
Member

Here's an example email:

https://github.com/fayton/cpython/commit/2d420b342509e6c2b597af82ea74c4cbb13e2abd
commit: 2d420b342509e6c2b597af82ea74c4cbb13e2abd
branch: refs/heads/3.5
author: cbiggles <[email protected]>
committer: Berker Peksag <[email protected]>
date: 2017-02-08T15:37:50+03:00
summary:

Update .gitignore
(cherry picked from commit 9d9ed0e5cceef45fd63dc1f7b3fe6e695da16e83)

files:
M .gitignore

diff --git a/.gitignore b/.gitignore
index c2b4fc7..e0d0685 100644
--- a/.gitignore
+++ b/.gitignore
@@ -93,3 +93,4 @@ htmlcov/
 Tools/msi/obj
 Tools/ssl/amd64
 Tools/ssl/win32
+foo

Differences with the Mercurial emails:

  • Diff stats are not included:

    files:
    M .gitignore
    

    instead of

    files:
    Lib/test/test_gdb.py |  24 +++++++-----------------
    1 files changed, 7 insertions(+), 17 deletions(-)
    
  • Added committer information

@berkerpeksag
Copy link
Member

I've just finished the first version of the webhook: https://github.com/berkerpeksag/cpython-emailer-webhook

@warsaw
Copy link
Member Author

warsaw commented Feb 13, 2017

Would it be too difficult to add a call to git diff --stat with the appropriate tweaks for readability?

@berkerpeksag
Copy link
Member

A call to git diff would require a clone of python/cpython. Current version of the webhook only uses GitHub API.

@berkerpeksag
Copy link
Member

berkerpeksag commented Jul 21, 2017

This can be closed now. With help of the infra team and Brett, it's now up! See https://mail.python.org/pipermail/python-checkins/2017-July/151274.html for an example email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants