Hey Andrew,
The intent was to be consistent with the way the merge messages look
before. But I agree it obfuscates the commit messages from the user
and hides them further down.
I think your proposal is good, but it might be better to use the title
of their pull request message rather than the first line of the most
recent commit in their branch (not sure what you meant by "commit
message").
Maybe you could submit a pull request for this? The script we use to
merge things is in dev/merge_spark_pr.py.
Another nice thing is if people are formatting their titles with
jira's then it will all look nice and pretty... which is kind of the
goal.
- Patrick
On Sun, Feb 9, 2014 at 11:55 PM, Andrew Ash <and...@andrewash.com> wrote:
> The current script for merging a GitHub PR squashes the commits and sticks a
> "Merge pull request #123 from abc/def" at the top of the commit message.
> However this obscures the original commit message when doing a short gitlog
> (first line only) so the recent history is much less meaningful than before.
>
> Compare recent history A:
>
> * 919bd7f Prashant Sharma 86 minutes ago (origin/master, origin/HEAD)Merge
> pull request #567 from ScrapCodes/style2.
> * 2182aa3 Martin Jaggi 8 hours ago Merge pull request #566 from
> martinjaggi/copy-MLlib-d.
> * afc8f3c qqsun8819 10 hours ago Merge pull request #551 from
> qqsun8819/json-protocol.
> * 94ccf86 Patrick Wendell 10 hours ago Merge pull request #569 from
> pwendell/merge-fixes.
> * b69f8b2 Patrick Wendell 14 hours ago Merge pull request #557 from
> ScrapCodes/style. Closes #557.
> * b6dba10 CodingCat 24 hours ago Merge pull request #556 from
> CodingCat/JettyUtil. Closes #556.
> | * de22abc jyotiska 24 hours ago (origin/branch-0.9)Merge pull request
> #562 from jyotiska/master. Closes #562.
> * | 2ef37c9 jyotiska 24 hours ago Merge pull request #562 from
> jyotiska/master. Closes #562.
> | * 2e3d1c3 Patrick Wendell 24 hours ago Merge pull request #560 from
> pwendell/logging. Closes #560.
> * | b6d40b7 Patrick Wendell 24 hours ago Merge pull request #560 from
> pwendell/logging. Closes #560.
> * | f892da8 Patrick Wendell 25 hours ago Merge pull request #565 from
> pwendell/dev-scripts. Closes #565.
> * | c2341c9 Mark Hamstra 32 hours ago Merge pull request #542 from
> markhamstra/versionBump. Closes #542.
> | * 22e0a3b Qiuzhuang Lian 35 hours ago Merge pull request #561 from
> Qiuzhuang/master. Closes #561.
> * | f0ce736 Qiuzhuang Lian 35 hours ago Merge pull request #561 from
> Qiuzhuang/master. Closes #561.
> * | 7805080 Jey Kottalam 35 hours ago Merge pull request #454 from
> jey/atomic-sbt-download. Closes #454.
> * | fabf174 Martin Jaggi 2 days ago Merge pull request #552 from
> martinjaggi/master. Closes #552.
> * | 3a9d82c Andrew Ash 3 days ago Merge pull request #506 from
> ash211/intersection. Closes #506.
> | * ce179f6 Andrew Or 3 days ago Merge pull request #533 from
> andrewor14/master. Closes #533.
>
>
> To B:
>
> If you go back some time in history, you get a much more branched history,
> like this:
>
> | * | | | | | | | | 0984647 Patrick Wendell 4 weeks ago Enable compression
> by default for spills
> |/ / / / / / / / /
> | * | | | | | | | 4e497db Tathagata Das 4 weeks ago Removed
> StreamingContext.registerInputStream and registerOutputStream - they were
> useless as InputDStream has been made to register itself. Also made DS
> * | | | | | | | | fdaabdc Patrick Wendell 4 weeks ago Merge pull request
> #380 from mateiz/py-bayes
> |\ \ \ \ \ \ \ \ \
> | | | | | * | | | | c2852cf Frank Dai 4 weeks ago Indent two spaces
> * | | | | | | | | | 4a805af Patrick Wendell 4 weeks ago Merge pull request
> #367 from ankurdave/graphx
> |\ \ \ \ \ \ \ \ \ \
> | * | | | | | | | | | 80e73ed Joseph E. Gonzalez 4 weeks ago Adding minimal
> additional functionality to EdgeRDD
> * | | | | | | | | | | 945fe7a Patrick Wendell 4 weeks ago Merge pull
> request #408 from pwendell/external-serializers
> |\ \ \ \ \ \ \ \ \ \ \
> | | * | | | | | | | | | 4bafc4f Joseph E. Gonzalez 4 weeks ago adding
> documentation about EdgeRDD
> * | | | | | | | | | | | 68641bc Patrick Wendell 4 weeks ago Merge pull
> request #413 from rxin/scaladoc
> |\ \ \ \ \ \ \ \ \ \ \ \
> | | | | | | | | * | | | | 12386b3 Frank Dai 4 weeks ago Since getLong() and
> getInt() have side effect, get back parentheses, and remove an empty line
> | | | | | | | | * | | | | 0d94d74 Frank Dai 4 weeks ago Code clean up for
> mllib
> * | | | | | | | | | | | | 0ca0d4d Patrick Wendell 4 weeks ago Merge pull
> request #401 from andrewor14/master
> |\ \ \ \ \ \ \ \ \ \ \ \ \
> | | | | * | | | | | | | | | af645be Ankur Dave 4 weeks ago Fix all code
> examples in guide
> | | | | * | | | | | | | | | 2cd9358 Ankur Dave 4 weeks ago Finish
> 6f6f8c928ce493357d4d32e46971c5e401682ea8
> * | | | | | | | | | | | | | 08b9fec Patrick Wendell 4 weeks ago Merge pull
> request #409 from tdas/unpersist
>
> Ignoring the merge commits here, the commit messages are much better here
> than in the current setup because they're what the original author wrote.
> Not a pretty generic "merged pull request #123 from ash211/patch5" or
> similar.
>
> Looking at one of those squashed commits, we can see the commit message:
>
> $ git show afc8f3c
> commit afc8f3cb9a7afe3249500a7d135b4a54bb3e58c4
> Author: qqsun8819 <jin....@alibaba-inc.com>
> Date: Sun Feb 9 13:57:29 2014 -0800
>
> Merge pull request #551 from qqsun8819/json-protocol.
>
> [SPARK-1038] Add more fields in JsonProtocol and add tests that verify
> the JSON itself
>
> This is a PR for SPARK-1038. Two major changes:
> 1 add some fields to JsonProtocol which is new and important to
> standalone-related data structures
> 2 Use Diff in liftweb.json to verity the stringified Json output for
> detecting someone mod type T to Option[T]
>
> Author: qqsun8819 <jin....@alibaba-inc.com>
>
> Closes #551 and squashes the following commits:
>
> fdf0b4e [qqsun8819] [SPARK-1038] 1. Change code style for more readable
> according to rxin review 2. change submitdate hard-coded string to a date
> object toString for more complexiblity
> 095a26f [qqsun8819] [SPARK-1038] mod according to review of pwendel,
> use hard-coded json string for json data validation. Each test use its own
> json string
> 0524e41 [qqsun8819] Merge remote-tracking branch 'upstream/master' into
> json-protocol
> d203d5c [qqsun8819] [SPARK-1038] Add more fields in JsonProtocol and add
> tests that verify the JSON itself
>
>
> I'd like to propose modifying the git merge/squash script to place that
> first line ("Merge pull request #551 from qqsun8819/json-protocol") farther
> down in the squashed commit message, to right above the "Closes #551 and
> squashes the following commits:" line.
>
> That way the author's original one-line commit message title remains intact.
>
> Thoughts?
>
> Thanks!
> Andrew
>
>
> P.S. These graphs are made with this hlog alias:
>
> hlog = log --date-order --all --graph --format=\"%C(green)%h%Creset
> %C(yellow)%an%Creset %C(blue bold)%ar%Creset %C(red bold)%d%Creset%s\"
