Email pull requests #284

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
7 participants
@dscho

dscho commented Apr 13, 2012

In the Git for Windows project, our workflow is more mailing list-centric than in other projects, so we would really love to get mails whenever somebody issues a pull request or opens/modifies an issue.

So here is an attempt to support that: the Email service hook is augmented by the check boxes "Send pull requests" and "Send issues", for backwards-compatibility, they are unchecked by default.

Please let me know if I did something wrong; if things look good, we could enhance the mail body and probably it would also be a good idea to add handlers for issue_comments, commit_comments, gollum, download, fork and fork_apply.

dscho added some commits Apr 13, 2012

email: refactor sending a message
The next commit will add additional handlers that want to send mails,
so let's refactor out that method.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
email: optionally send issues and pull requests, too
For backwards-compatibility, those are opt-in.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@rtomayko

This comment has been minimized.

Show comment
Hide comment
@rtomayko

rtomayko Apr 13, 2012

Contributor

Not saying we won't accept this but you'd probably be better off creating a user with the mailing list's email address and adding them to the repository so they get all issues / pull request emails.

Contributor

rtomayko commented Apr 13, 2012

Not saying we won't accept this but you'd probably be better off creating a user with the mailing list's email address and adding them to the repository so they get all issues / pull request emails.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Apr 13, 2012

I actually thought so, too. So I added such a user.

The big problem is that the password emails go to the same address. Big bummer, as the user needs sufficient permissions to wreak havoc.

Second problem is that the link sent with those notifications (as well as the reply-to address) know that the notification was sent to that user. As a consequence, every single reply is attributed to that user rather than the real author of the reply.

So while it is a cute idea, adding a dedicated user with the public mailing list's address is unfortunately not good enough...

dscho commented Apr 13, 2012

I actually thought so, too. So I added such a user.

The big problem is that the password emails go to the same address. Big bummer, as the user needs sufficient permissions to wreak havoc.

Second problem is that the link sent with those notifications (as well as the reply-to address) know that the notification was sent to that user. As a consequence, every single reply is attributed to that user rather than the real author of the reply.

So while it is a cute idea, adding a dedicated user with the public mailing list's address is unfortunately not good enough...

@rtomayko

This comment has been minimized.

Show comment
Hide comment
@rtomayko

rtomayko Apr 13, 2012

Contributor

That's super interesting. I'm going to see what we can do to make the normal emails more mailing list friendly.

Contributor

rtomayko commented Apr 13, 2012

That's super interesting. I'm going to see what we can do to make the normal emails more mailing list friendly.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Apr 13, 2012

I fear if you make them more mailing list friendly, then all the advantages for regular users will be gone. For example, it is a super-cool thing that you can reply to notifications and the replies are connected with your GitHub account (which might not know the mail address from which you reply).

Also: just adding an account to use it only for notifications (something that the GitHub API and the Service Hooks clearly were designed for) appears to me as the wrong thing. It would incur a serious technical debt to violate the separation of concerns so blatantly...

So I would like to ask you not to change the mails being sent to GitHub accounts. Pretty please. I really like them the way they are.

And if you don't want the Email Service Hook to be able to notify via mail of anything else than push events, please say so now. Because then I can avoid beating on a dead horse by pouring more time into the issue.

dscho commented Apr 13, 2012

I fear if you make them more mailing list friendly, then all the advantages for regular users will be gone. For example, it is a super-cool thing that you can reply to notifications and the replies are connected with your GitHub account (which might not know the mail address from which you reply).

Also: just adding an account to use it only for notifications (something that the GitHub API and the Service Hooks clearly were designed for) appears to me as the wrong thing. It would incur a serious technical debt to violate the separation of concerns so blatantly...

So I would like to ask you not to change the mails being sent to GitHub accounts. Pretty please. I really like them the way they are.

And if you don't want the Email Service Hook to be able to notify via mail of anything else than push events, please say so now. Because then I can avoid beating on a dead horse by pouring more time into the issue.

Provide nicer mail bodies for issues and pull requests
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

@dscho dscho closed this Apr 14, 2012

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Apr 14, 2012

That last commit is not tested at all since I ran out of time budget for this issue. However, I did not want to leave it in an awkward state, in spite of the reluctance to tell me how to move forward.

In the meantime I made another solution and that's it for me. It is just a great pity that everybody and her dog has to make their own solutions for mailing out issues and pull requests. Whatever.

dscho commented Apr 14, 2012

That last commit is not tested at all since I ran out of time budget for this issue. However, I did not want to leave it in an awkward state, in spite of the reluctance to tell me how to move forward.

In the meantime I made another solution and that's it for me. It is just a great pity that everybody and her dog has to make their own solutions for mailing out issues and pull requests. Whatever.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Apr 17, 2012

It has been pointed out to me that it is not obvious without context why I closed this pull request. After all, quite a number of people have indicated to me in private (and some even in public) that they would love to have the option of enabling an Email hook for reporting issues and pull requests in addition to pushes, without the need for an own, external server and a lot of configuration.

So here is my explanation of the context:

A couple of months ago, I asked a Hubbernaut who offered his help publicly for all issues relating to GitHub whether there was any chance to get what I need: email notifications about pull requests.

The following workarounds were suggested:

  • registering the mailing list as a member; but I quickly found out that this is a gaping security hole,
  • adding an external email account, registering it as a member; but again I found out pretty quickly that this is a problem because of the convenience in emails that automatically connects replies with the GitHub account the mail was sent to -- a convenience I definitely do not want to go away! Hence my terror at seeing this suggested earlier in this issue,
  • trying to find a way to teach Hotmail to transform mails before forwarding; but there is no option to do so,
  • trying to find another mail hoster allowing that; but I did not find any.

After implementing all that I was at my wits' end for a while.

Then I found out that github-services could handle more than just push events. I have to admit that I was pretty disappointed that I had to stumble upon this, instead of being pointed to it, after asking an insider to help me.

So I asked for help in the msysGit community (for which those notifications are intended) because I did not speak Ruby. To make it short: the users of Git for Windows are not known to volunteer much else than less-than-flattering comments and demands for others to scratch their itches. Certainly no programming for free, why should they do that?

I finally decided to learn enough Ruby to do it myself. It turned out to be an easier exercise than I hoped, Ruby is quite a nice language, although I am sure that there are enough issues with my coding that a Ruby expert could help me with.

In total, I had already invested a lot of time into investigating and trying to solve this issue. But that coding came along nicely, I even managed to write and run the corresponding tests to make sure that my changes were not completely wrong.

So I was happy: after such a lot of work -- only a small part of which being the thing I like most: coding -- I finally saw the end of the tunnel. I opened this pull request. Only a few more steps to go, and I would finally have what I need and could go back to fun stuff.

Just imagine how frustrating that first comment was: ''Not saying we won't accept this ...''. I am German, used to direct communication (which sometimes comes over as rude to non-Germans), so I had to learn quite a bit to be able to read between the lines when Americans try to tell me something politely and therefore phrase it much more positively than they mean it. Therefore my assumption might be wrong that nobody in GitHub was willing to 1) integrate my changes, or 2) work with me to improve my code so it could be merged. That was my interpretation, though.

In the end, I decided to avoid the Bus Stop problem and just write off the time I spent so far. I invested another 20+ hours (way less than I had spent before on this frustrating business) to write a .php script to subscribe to the events I so dearly want to be forwarded to the mailing list.

Now I have to abuse a server that was definitely not meant for that job. But at least it works. And after writing this comment, I will try not to waste any more of my time on this issue.

Do not get me wrong: I really, really, really like GitHub. That is the reason why the disappointment sits so deep.

dscho commented Apr 17, 2012

It has been pointed out to me that it is not obvious without context why I closed this pull request. After all, quite a number of people have indicated to me in private (and some even in public) that they would love to have the option of enabling an Email hook for reporting issues and pull requests in addition to pushes, without the need for an own, external server and a lot of configuration.

So here is my explanation of the context:

A couple of months ago, I asked a Hubbernaut who offered his help publicly for all issues relating to GitHub whether there was any chance to get what I need: email notifications about pull requests.

The following workarounds were suggested:

  • registering the mailing list as a member; but I quickly found out that this is a gaping security hole,
  • adding an external email account, registering it as a member; but again I found out pretty quickly that this is a problem because of the convenience in emails that automatically connects replies with the GitHub account the mail was sent to -- a convenience I definitely do not want to go away! Hence my terror at seeing this suggested earlier in this issue,
  • trying to find a way to teach Hotmail to transform mails before forwarding; but there is no option to do so,
  • trying to find another mail hoster allowing that; but I did not find any.

After implementing all that I was at my wits' end for a while.

Then I found out that github-services could handle more than just push events. I have to admit that I was pretty disappointed that I had to stumble upon this, instead of being pointed to it, after asking an insider to help me.

So I asked for help in the msysGit community (for which those notifications are intended) because I did not speak Ruby. To make it short: the users of Git for Windows are not known to volunteer much else than less-than-flattering comments and demands for others to scratch their itches. Certainly no programming for free, why should they do that?

I finally decided to learn enough Ruby to do it myself. It turned out to be an easier exercise than I hoped, Ruby is quite a nice language, although I am sure that there are enough issues with my coding that a Ruby expert could help me with.

In total, I had already invested a lot of time into investigating and trying to solve this issue. But that coding came along nicely, I even managed to write and run the corresponding tests to make sure that my changes were not completely wrong.

So I was happy: after such a lot of work -- only a small part of which being the thing I like most: coding -- I finally saw the end of the tunnel. I opened this pull request. Only a few more steps to go, and I would finally have what I need and could go back to fun stuff.

Just imagine how frustrating that first comment was: ''Not saying we won't accept this ...''. I am German, used to direct communication (which sometimes comes over as rude to non-Germans), so I had to learn quite a bit to be able to read between the lines when Americans try to tell me something politely and therefore phrase it much more positively than they mean it. Therefore my assumption might be wrong that nobody in GitHub was willing to 1) integrate my changes, or 2) work with me to improve my code so it could be merged. That was my interpretation, though.

In the end, I decided to avoid the Bus Stop problem and just write off the time I spent so far. I invested another 20+ hours (way less than I had spent before on this frustrating business) to write a .php script to subscribe to the events I so dearly want to be forwarded to the mailing list.

Now I have to abuse a server that was definitely not meant for that job. But at least it works. And after writing this comment, I will try not to waste any more of my time on this issue.

Do not get me wrong: I really, really, really like GitHub. That is the reason why the disappointment sits so deep.

@jcsogo

This comment has been minimized.

Show comment
Hide comment
@jcsogo

jcsogo May 31, 2012

Oh! That is a pity. We are in the process to move darktable to GitHub and I never thought this would be a problem. Please, enable this, or the nice pull request feature would became almost useless.

jcsogo commented May 31, 2012

Oh! That is a pity. We are in the process to move darktable to GitHub and I never thought this would be a problem. Please, enable this, or the nice pull request feature would became almost useless.

@g2p

This comment has been minimized.

Show comment
Hide comment
@g2p

g2p Jun 6, 2013

@rtomayko, it's a shame you won't do this.

Reviews are part of a project's institutional knowledge, they need to be sent to a mailing list where everyone will see them.
I know several projects who outright refuse pull requests because of this.

g2p commented Jun 6, 2013

@rtomayko, it's a shame you won't do this.

Reviews are part of a project's institutional knowledge, they need to be sent to a mailing list where everyone will see them.
I know several projects who outright refuse pull requests because of this.

@rtomayko

This comment has been minimized.

Show comment
Hide comment
@rtomayko

rtomayko Jun 15, 2013

Contributor

Sorry, I hadn't seen @dscho reply until just now. I saw that the issue was closed and assumed a different solution was being pursued.

Just imagine how frustrating that first comment was: ''Not saying we won't accept this ...''. I am German, used to direct communication (which sometimes comes over as rude to non-Germans), so I had to learn quite a bit to be able to read between the lines when Americans try to tell me something politely and therefore phrase it much more positively than they mean it.

That was actually not a read between the lines type of statement at all. "Not saying we won't accept this" meant exactly that - I didn't want my comment to deter from pursuing the hook solution in this PR further. I was simply suggesting that trying an alternative approach may be a quicker solution for people that were trying to get something working here. While it's true that Americans sometimes try to soften communications, it's pretty rare that they mean the opposite of what is stated, especially in online communication.

I'm open to accepting a patch here. I certainly agree that some kind of ML friendly system for pull requests makes a lot of sense. If this diff is still in good shape, feel free to reopen. After a quick pass the only thing that seems to be missing is maybe options for whether messages should be sent for issues/PRs in addition to commits since I don't think everyone is going to want that behavior. I'm also curious how closely the PR messages mimic git-request-pull(1) format, which should probably be emulated as closely as possible.

Contributor

rtomayko commented Jun 15, 2013

Sorry, I hadn't seen @dscho reply until just now. I saw that the issue was closed and assumed a different solution was being pursued.

Just imagine how frustrating that first comment was: ''Not saying we won't accept this ...''. I am German, used to direct communication (which sometimes comes over as rude to non-Germans), so I had to learn quite a bit to be able to read between the lines when Americans try to tell me something politely and therefore phrase it much more positively than they mean it.

That was actually not a read between the lines type of statement at all. "Not saying we won't accept this" meant exactly that - I didn't want my comment to deter from pursuing the hook solution in this PR further. I was simply suggesting that trying an alternative approach may be a quicker solution for people that were trying to get something working here. While it's true that Americans sometimes try to soften communications, it's pretty rare that they mean the opposite of what is stated, especially in online communication.

I'm open to accepting a patch here. I certainly agree that some kind of ML friendly system for pull requests makes a lot of sense. If this diff is still in good shape, feel free to reopen. After a quick pass the only thing that seems to be missing is maybe options for whether messages should be sent for issues/PRs in addition to commits since I don't think everyone is going to want that behavior. I'm also curious how closely the PR messages mimic git-request-pull(1) format, which should probably be emulated as closely as possible.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Jun 15, 2013

Sorry, at this point I am no longer willing to spend an effort on this issue.

dscho commented Jun 15, 2013

Sorry, at this point I am no longer willing to spend an effort on this issue.

@sschuberth

This comment has been minimized.

Show comment
Hide comment
@sschuberth

sschuberth Aug 26, 2014

@dscho Any chance you could reopen this? I'm constantly missing PRs for mingwGitDevEnv because I do not get notified by mail about them. Maybe @mhagger can help pushing this forward? As a work-around, I'm now using IFTTT on my Android device to send me mails about new PRs.

@dscho Any chance you could reopen this? I'm constantly missing PRs for mingwGitDevEnv because I do not get notified by mail about them. Maybe @mhagger can help pushing this forward? As a work-around, I'm now using IFTTT on my Android device to send me mails about new PRs.

@dscho

This comment has been minimized.

Show comment
Hide comment
@dscho

dscho Sep 3, 2014

@sschuberth sorry, at this point, GitHub would have to pay me to work on this issue. I still have three other projects I have to finish for them before that, though (struggling to find the energy and time).

dscho commented Sep 3, 2014

@sschuberth sorry, at this point, GitHub would have to pay me to work on this issue. I still have three other projects I have to finish for them before that, though (struggling to find the energy and time).

@bluerise

This comment has been minimized.

Show comment
Hide comment
@bluerise

bluerise Oct 8, 2014

I was just looking for the same feature and am disappointed to not see it supported. I would really love to be able to only select specific events for the email. It'd be great for Bitrig.

What was the issue with the initial solution? Is there any other way?

bluerise commented Oct 8, 2014

I was just looking for the same feature and am disappointed to not see it supported. I would really love to be able to only select specific events for the email. It'd be great for Bitrig.

What was the issue with the initial solution? Is there any other way?

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