git-notifier icon indicating copy to clipboard operation
git-notifier copied to clipboard

an option to skip fast-forward master merge emails

Open stas00 opened this issue 7 years ago • 10 comments

In a busy project with many commits we get a lot of fast-forward master merges (not branch merges).

Merge branch 'master' of github.com:...

which generates a lot of emails which are just noise.

Would it be possible to add an option to not send emails for such events?

Thank you.

stas00 avatar Sep 28 '18 05:09 stas00

What's the subject of these emails? Is it master's head updated: ...? Mind pasting one mail in here?

rsmmr avatar Sep 28 '18 14:09 rsmmr

Yes, of course, the subject's post-[prefix] master: part:

Merge branch 'master' of github.com:fastai/fastai_pytorch (fb618de)

The body:

Repository : https://github.com/fastai/fastai_pytorch
On branch  : master
Link       : https://github.com/fastai/fastai_pytorch/compare/4b735ffc71dee3d19ca1be8cc7c6dda465d3e0c0...fb618de14c0354b3a24f02e573ab8879caf149ac

> ---------------------------------------------------------------

commit fb618de14c0354b3a24f02e573ab8879caf149ac
Merge: 8d0c2d0 4b735ff
Author: Stas Bekman <[email protected]>
Date:   Thu Sep 27 21:59:41 2018 -0700

    Merge branch 'master' of github.com:fastai/fastai_pytorch



> ---------------------------------------------------------------

fb618de14c0354b3a24f02e573ab8879caf149ac
 docs/basic_train.html            |  271 ++++++++++-
 docs_src/basic_train.ipynb       | 1002 +++++++++++++++++++++++++++++++-------
 docs_src/tools/sgen_notebooks.py |    5 +
 fastai/gen_doc/__init__.py       |    2 +-
 fastai/gen_doc/gen_notebooks.py  |   50 +-
 fastai/gen_doc/nbdoc.py          |    2 +-
 fastai/gen_doc/sgen_notebooks.py |   55 ---
 7 files changed, 1127 insertions(+), 260 deletions(-)

stas00 avatar Sep 28 '18 15:09 stas00

I guess there can be an actual change in one of the those situations, can it be? So far, it's always just the fast-forward w/o any changes to the code base.

stas00 avatar Sep 28 '18 15:09 stas00

This is an actual merge commit, not a fast-forward. I was asking because a fast-forward would generate a different kind of a mail ("head updated ..."). And yes, a merge commit can have changes of its own coming with it, which the email would show. We could add an option to suppress merge commits, but not sure that would be very useful, merging a branch seems like an operation that be reported just like any other change.

rsmmr avatar Sep 29 '18 16:09 rsmmr

Well, I call it fast-forward since when I do git pull before git commit, it goes:

$ git pull
remote: Enumerating objects: 552, done.
remote: Counting objects: 100% (552/552), done.
remote: Compressing objects: 100% (50/50), done.
remote: Total 1242 (delta 507), reused 524 (delta 502), pack-reused 690
Receiving objects: 100% (1242/1242), 49.60 MiB | 7.97 MiB/s, done.
Resolving deltas: 100% (974/974), completed with 121 local objects.
From github.com:fastai/fastai_pytorch
   7cd350c..ffef0b4  master     -> origin/master
Updating 7cd350c..ffef0b4
Fast-forward

but if I start from the same situation and I did:

$ git commit ...
$ git push

it forces me to git pull due to changes in origin. So next I go:

$ git pull

and this causes merge which triggers this pointless email I'm trying to skip. I call it pointless because so far none of these included any merge events that carried changes in them. It's just an email with an indication of what files were pulled and merged in. (But they could include an actual code change)

You're correct that technically this is no longer fast-forward, but in reality it is most of the time.

I'm still new to git, so I don't yet know the correct terminology, but basically how can I skip emails about events that don't carry any actual modifications to the source code in them?

And the real practical reason I'm trying to eliminate these (other than noise) is because we are dealing with 100/email day limit on a free smtp-relay. And I see about 25% of emails are of this kind, since most devs try to push and discover that things have changed up stream, whereas if they always did pull first there would have been a lot less of those.

stas00 avatar Sep 29 '18 17:09 stas00

git pull --rebase avoids the additional merge, but yeah, I get your point. I can see adding an option that suppresses mails without any diff content. Might be as simple as changing sendChangeMail() to abort if the size of the git diff output is zero, but would need to try it.

rsmmr avatar Sep 29 '18 17:09 rsmmr

git pull --rebase is not an option, since the project includes many jupyter notebooks with nbstripout filter, I tried this approach and it just blows things up (everything collides). Plus, it seems that I landed in the 'merge, not rebase' camp, but that's unrelated to the issue at hand.

Your suggestion on how to implement it sounds perfect to me, as it answers my quest exactly. Don't send an email unless it carries a diff in it.

Except, I have just remembered that there is one more kind of emails w/o diff that is important to preserve, and that's when a branch is merged into master - e.g. the most recent one - and it triggered 2 emails, none with diff:

Subject: fastai: Merge branch 'package' (pypi and conda build, +fastai.__version__) (e1a9857)
Repository : https://github.com/fastai/fastai_pytorch
On branch  : master
Link       : https://github.com/fastai/fastai_pytorch/compare/feae73216b36e27effffd649664c533dc628385f...e1a98577a542c7c4485a7cece629de00d0510dc9

> ---------------------------------------------------------------

commit e1a98577a542c7c4485a7cece629de00d0510dc9
Merge: feae732 bb8714a
Author: Stas Bekman <[email protected]>
Date:   Wed Sep 26 19:03:52 2018 -0700

    Merge branch 'package' (pypi and conda build, +fastai.__version__)
> ---------------------------------------------------------------

e1a98577a542c7c4485a7cece629de00d0510dc9
 DEVELOPERS.md        | 267 +++++++++++++++++++++++++++++++++++++++++++++++++++
 conda/meta.yaml      |  81 ++++++++++++++++
 fastai/__init__.py   |   2 +-
 fastai/version.py    |   1 +
 requirements.txt     |   1 +
 requirements_dev.txt |   2 +-
 setup.cfg            |   4 +-
 setup.py             |  35 +++++--
 tests/test_core.py   |  72 ++++++++++++++
 tests/test_fastai.py |   5 +-
 10 files changed, 456 insertions(+), 14 deletions(-)
Subject: fastai: master's head updated: Merge branch 'package' (pypi and conda build, +fastai.__v ...
Repository : https://github.com/fastai/fastai_pytorch
Branch 'master' now includes:

    9dea2d0 include all modules under fastai/
    3e057e9 some initial tests
    22040af autogenerate fastai/version.py
    8dbc544 experiment with test parametrizing
    47bc73d package requirements
    12a4b52 fix requirements to use >= instead of ==
    1f0d870 simplify requirement discovery code
    d0f0cec versioning
    01e43b8 build docs - WIP
    2c86bcb configuration
    6cd590e pypi test is done
    1a8be90 repo rename
    4b73cf2 WIP on conda build
    e4c7595 improve/re-org build docs
    bb8714a tweaking build scripts
    e1a9857 Merge branch 'package' (pypi and conda build, +fastai.__version__)

Why wasn't a diff sent for this event in first place when master merged this branch?

I configured git-notifier to only email 'master' changes, so it did not send these diffs already via the branch.

Perhaps this is a separate issue.

stas00 avatar Sep 29 '18 17:09 stas00

(So git pull --rebase leads to conflicts, but git pull can do a clean merge here? Interesting.)

Now I'm getting confused about what you'd like to keep exactly. That 2nd email is a fast-forward now: Just the head's position changes, no commit at all. The 1st looks very similar to your earlier example, I'm not sure there's a good way to distinguish the two.

rsmmr avatar Sep 30 '18 15:09 rsmmr

(So git pull --rebase leads to conflicts, but git pull can do a clean merge here? Interesting.)

I think the problem is that nbstripout is not a symmetrical content filter (it strips out some json data on check in, but nothing gets changed on checkout). I don't think git is well designed for this. It's very helpful with collaborating on jupyter notebooks, but it comes with side-effects during merging. There is a whole tool nbdime designed just for diffing/merging jupyter notebooks. That's why I was asking for --ext-diff in PR https://github.com/rsmmr/git-notifier/pull/25.

Now I'm getting confused about what you'd like to keep exactly. That 2nd email is a fast-forward now: Just the head's position changes, no commit at all. The 1st looks very similar to your earlier example, I'm not sure there's a good way to distinguish the two.

So the quest again is: show all actual changes to the master branch, ideally avoiding any unnecessary emails. Does this make sense as a goal?

All was good and clear, but then I noticed that I have never seen any commit emails when a branch is merged into master, other than a notification that a merge happened. So before we can figure out what I'd like to keep, in my last comment I was asking where are the diffs emails for this branch to master merge? the merge changed the master's contents, but no diffs were sent. I hope my intention is more clear now.

stas00 avatar Sep 30 '18 18:09 stas00

ping

stas00 avatar Oct 09 '18 21:10 stas00