Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Edward Hervey
Administrator
Thomas,

  You were already told off once about merges. You now do a
double-merge... of a one commit branch.

  Please, CHECK CHECK CHECK and CHECK before pushing (use a graphical
tool like giggle) that you're not doing a non-fast-forward merge (you
shouldn't see any branches in giggle before pushing but only a straight
line), especially when you are not familiar with git.

   Edward

On Wed, 2009-04-15 at 13:43 -0700, Thomas Vander Stichele wrote:

> Module: gst-python
> Branch: master
> Commit: 62d82ad5b64974dc2be8b9684716c2144c3c9371
> URL:    http://cgit.freedesktop.org/gstreamer/gst-python/commit/?id=62d82ad5b64974dc2be8b9684716c2144c3c9371
>
> Author: Thomas Vander Stichele <thomas (at) apestaart (dot) org>
> Date:   Wed Apr 15 22:38:28 2009 +0200
>
> Merge branch 'bz-577735'
>
> ---
>
>
>
>
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by:
> High Quality Requirements in a Collaborative Environment.
> Download a free trial of Rational Requirements Composer Now!
> http://p.sf.net/sfu/www-ibm-com
> _______________________________________________
> gstreamer-cvs mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/gstreamer-cvs


------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Thomas Vander Stichele
On Thu, 2009-04-16 at 00:42 +0200, Edward Hervey wrote:
> Thomas,
>
>   You were already told off once about merges. You now do a
> double-merge... of a one commit branch.

Your advice will go down better if you stop pretending you're a parent
talking to a child.  Especially if you do it in public.

I don't know why you think it's a one commit branch, it wasn't.  The
commit is squashed.


>   Please, CHECK CHECK CHECK and CHECK before pushing (use a graphical
> tool like giggle) that you're not doing a non-fast-forward merge (you
> shouldn't see any branches in giggle before pushing but only a straight
> line), especially when you are not familiar with git.

I did exactly that, asking for help as I did so to thaytan and ds.
Thaytan said on IRC that the commit looked fine to him.  AFAICT the
commit looks like a single commit; the URL shows it as such.  Feel free
to explain to me how I can see from that URL that I did it wrong.

By the way, I did *exactly* what you said in your last 'telling off'
mail:

  * git rebase master
  * git checkout master
  * git merge whatever

So if something is missing from this list, feel free to let me know, and
document it somewhere publically so people have something to look at
when they are really trying to do the right thing.

Constructive feedback is welcome.

Thomas



>    Edward
>
> On Wed, 2009-04-15 at 13:43 -0700, Thomas Vander Stichele wrote:
> > Module: gst-python
> > Branch: master
> > Commit: 62d82ad5b64974dc2be8b9684716c2144c3c9371
> > URL:    http://cgit.freedesktop.org/gstreamer/gst-python/commit/?id=62d82ad5b64974dc2be8b9684716c2144c3c9371
> >
> > Author: Thomas Vander Stichele <thomas (at) apestaart (dot) org>
> > Date:   Wed Apr 15 22:38:28 2009 +0200
> >
> > Merge branch 'bz-577735'
> >
> > ---
> >
> >
> >
> >
> > ------------------------------------------------------------------------------
> > This SF.net email is sponsored by:
> > High Quality Requirements in a Collaborative Environment.
> > Download a free trial of Rational Requirements Composer Now!
> > http://p.sf.net/sfu/www-ibm-com
> > _______________________________________________
> > gstreamer-cvs mailing list
> > [hidden email]
> > https://lists.sourceforge.net/lists/listinfo/gstreamer-cvs
>

--
Being hung over is like winning the lottery, except they pay you in
regret!
--
GStreamer - bringing multimedia to your desktop
http://gstreamer.freedesktop.org/



------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Edward Hervey
Administrator
On Thu, 2009-04-16 at 01:07 +0200, Thomas Vander Stichele wrote:
> On Thu, 2009-04-16 at 00:42 +0200, Edward Hervey wrote:
> > Thomas,
> >
> >   You were already told off once about merges. You now do a
> > double-merge... of a one commit branch.
>
> Your advice will go down better if you stop pretending you're a parent
> talking to a child.  Especially if you do it in public.

  I'm sorry you take it that way, public mail or not.

>
> I don't know why you think it's a one commit branch, it wasn't.  The
> commit is squashed.
>
>
> >   Please, CHECK CHECK CHECK and CHECK before pushing (use a graphical
> > tool like giggle) that you're not doing a non-fast-forward merge (you
> > shouldn't see any branches in giggle before pushing but only a straight
> > line), especially when you are not familiar with git.
>
> I did exactly that, asking for help as I did so to thaytan and ds.
> Thaytan said on IRC that the commit looked fine to him.  AFAICT the
> commit looks like a single commit; the URL shows it as such.  Feel free
> to explain to me how I can see from that URL that I did it wrong.
>
> By the way, I did *exactly* what you said in your last 'telling off'
> mail:
>
>   * git rebase master
>   * git checkout master
>   * git merge whatever
>

  That should have produced a fast-forward merge (i.e. without a merge
commit).

  What I see in giggle lets me believe you did:
  * git merge master (onto your bz-577735 branch) (as opposed to git
rebase master)
  * git checkout master
  * git merge bz-577735

  I just rechecked with the branches at the following status:
  * bz-577735 : f99e67aa24878d26dbd7f494f310bb466c5bd1cd
  * master : bbedab4e6521fe7c813f23698fe650203b1d0820
  And if you do the commands mentionned above or in the wiki... it does
a fast-forward merge.


> So if something is missing from this list, feel free to let me know, and
> document it somewhere publically so people have something to look at
> when they are really trying to do the right thing.

  I think http://gstreamer.freedesktop.org/wiki/GitDeveloperGuidelines
already has the needed information (see Merging branches and the first
line of Pushing changes), and virtually nobody seems to do it wrong in
those regards except in very rare occasions (most of them being the
first time people use git).

>
> Constructive feedback is welcome.
>
> Thomas
>
>
>
> >    Edward
> >
> > On Wed, 2009-04-15 at 13:43 -0700, Thomas Vander Stichele wrote:
> > > Module: gst-python
> > > Branch: master
> > > Commit: 62d82ad5b64974dc2be8b9684716c2144c3c9371
> > > URL:    http://cgit.freedesktop.org/gstreamer/gst-python/commit/?id=62d82ad5b64974dc2be8b9684716c2144c3c9371
> > >
> > > Author: Thomas Vander Stichele <thomas (at) apestaart (dot) org>
> > > Date:   Wed Apr 15 22:38:28 2009 +0200
> > >
> > > Merge branch 'bz-577735'
> > >
> > > ---
> > >
> > >
> > >
> > >
> > > ------------------------------------------------------------------------------
> > > This SF.net email is sponsored by:
> > > High Quality Requirements in a Collaborative Environment.
> > > Download a free trial of Rational Requirements Composer Now!
> > > http://p.sf.net/sfu/www-ibm-com
> > > _______________________________________________
> > > gstreamer-cvs mailing list
> > > [hidden email]
> > > https://lists.sourceforge.net/lists/listinfo/gstreamer-cvs
> >
>


------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Thomas Vander Stichele
> > I did exactly that, asking for help as I did so to thaytan and ds.
> > Thaytan said on IRC that the commit looked fine to him.  AFAICT the
> > commit looks like a single commit; the URL shows it as such.  Feel free
> > to explain to me how I can see from that URL that I did it wrong.
> >
> > By the way, I did *exactly* what you said in your last 'telling off'
> > mail:
> >
> >   * git rebase master
> >   * git checkout master
> >   * git merge whatever
> >
>
>   That should have produced a fast-forward merge (i.e. without a merge
> commit).

What might have happened (I cannot tell for sure from my history) is
that I was already on the master branch when entering these commands.
Would that explain the commit history ?

>
>   What I see in giggle lets me believe you did:
>   * git merge master (onto your bz-577735 branch) (as opposed to git
> rebase master)

I definately did that at some point (it's also not clear to me why that
is wrong), but not yesterday before commiting.

>   * git checkout master
>   * git merge bz-577735
>
>   I just rechecked with the branches at the following status:
>   * bz-577735 : f99e67aa24878d26dbd7f494f310bb466c5bd1cd
>   * master : bbedab4e6521fe7c813f23698fe650203b1d0820
>   And if you do the commands mentionned above or in the wiki... it does
> a fast-forward merge.
>
>
> > So if something is missing from this list, feel free to let me know, and
> > document it somewhere publically so people have something to look at
> > when they are really trying to do the right thing.
>
>   I think http://gstreamer.freedesktop.org/wiki/GitDeveloperGuidelines
> already has the needed information (see Merging branches and the first
> line of Pushing changes), and virtually nobody seems to do it wrong in
> those regards except in very rare occasions (most of them being the
> first time people use git).

I think those instructions are very unclear.  Actual commands would be
helpful.

For example:

avoid 'merge commits':
      * rebase your branch to master before merging it into master:
        - git checkout branch
        - git rebase master
        - git checkout master
        - git merge branch
      * or do git rebase origin/master on master after merging - this
        should get rid of any merge commits:
        - git checkout master
        - git merge branch
        - git rebase origin/master
       
      * you might also want to use git pull --rebase instead of git pull
       
This last point seems out of place here; I have no idea at what point I
would be doing either a git pull or git pull --rebase.

If these are the correct instructions for each scenario, let me know,
and I'll update the wiki.

Finally, some explanation on how to verify the push you're about to do
would be helpful.

git push --dry-run is unhelpful to me - it doesn't show me what's about
to go in.
gitk can be confusing too if you don't understand all the UI bits.
A simple command that would say 'here are the x commits you are about to
push, plus their commit messages, plus their content' would be awesome.
I'm sure it's in there somewhere.

Lastly, it sounds to me that, if we don't want non-fast-forward-merge
commits to happen, the tools should just not allow you to, no ? If we
can make sure people don't commit wrongly indented code, why can't we
make sure they don't do
random-abracadabra-screwup-that-git-allows-but-we-don't-want ?

Thomas

--
Secrets... are not my concern.
Keeping them, is.
--
Elisa - future TV today !
http://elisa.fluendo.com/



------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Andy Wingo
In reply to this post by Edward Hervey
Heya Edward,

On Thu 16 Apr 2009 08:38, Edward Hervey <[hidden email]> writes:

> On Thu, 2009-04-16 at 01:07 +0200, Thomas Vander Stichele wrote:
>> Your advice will go down better if you stop pretending you're a parent
>> talking to a child.  Especially if you do it in public.
>
>   I'm sorry you take it that way, public mail or not.

Thomas is right, you know.

Andy
--
http://wingolog.org/

------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|

Re: [gst-cvs] gst-python: Merge branch 'bz-577735'

Edward Hervey
Administrator
In reply to this post by Thomas Vander Stichele
On Thu, 2009-04-16 at 12:32 +0200, Thomas Vander Stichele wrote:

> > > I did exactly that, asking for help as I did so to thaytan and ds.
> > > Thaytan said on IRC that the commit looked fine to him.  AFAICT the
> > > commit looks like a single commit; the URL shows it as such.  Feel free
> > > to explain to me how I can see from that URL that I did it wrong.
> > >
> > > By the way, I did *exactly* what you said in your last 'telling off'
> > > mail:
> > >
> > >   * git rebase master
> > >   * git checkout master
> > >   * git merge whatever
> > >
> >
> >   That should have produced a fast-forward merge (i.e. without a merge
> > commit).
>
> What might have happened (I cannot tell for sure from my history) is
> that I was already on the master branch when entering these commands.
> Would that explain the commit history ?

  Yes, that would most likely be the reason for those (causing the
branch you want to merge to not be rebased).

>
> >
> >   What I see in giggle lets me believe you did:
> >   * git merge master (onto your bz-577735 branch) (as opposed to git
> > rebase master)
>
> I definately did that at some point (it's also not clear to me why that
> is wrong), but not yesterday before commiting.

  Ah, well use it before committing and pushing then. Doing it early on
doesn't help in this scenario (but still, it does give you a nice
graphical view).

>
> >   * git checkout master
> >   * git merge bz-577735
> >
> >   I just rechecked with the branches at the following status:
> >   * bz-577735 : f99e67aa24878d26dbd7f494f310bb466c5bd1cd
> >   * master : bbedab4e6521fe7c813f23698fe650203b1d0820
> >   And if you do the commands mentionned above or in the wiki... it does
> > a fast-forward merge.
> >
> >
> > > So if something is missing from this list, feel free to let me know, and
> > > document it somewhere publically so people have something to look at
> > > when they are really trying to do the right thing.
> >
> >   I think http://gstreamer.freedesktop.org/wiki/GitDeveloperGuidelines
> > already has the needed information (see Merging branches and the first
> > line of Pushing changes), and virtually nobody seems to do it wrong in
> > those regards except in very rare occasions (most of them being the
> > first time people use git).
>
> I think those instructions are very unclear.  Actual commands would be
> helpful.
>
> For example:
>
> avoid 'merge commits':
>       * rebase your branch to master before merging it into master:
> - git checkout branch
> - git rebase master
> - git checkout master
> - git merge branch
>       * or do git rebase origin/master on master after merging - this
>         should get rid of any merge commits:
> - git checkout master
> - git merge branch
> - git rebase origin/master
>        
>       * you might also want to use git pull --rebase instead of git pull
>        
> This last point seems out of place here; I have no idea at what point I
> would be doing either a git pull or git pull --rebase.

  git pull is the shortcut for:
  * git fetch
  * git merge
  Whereas git pull --rebase is the shortcut for:
  * git fetch
  * git rebase

  The remote branch (origin/master) after git fetch is no different than
a local branch.

>
> If these are the correct instructions for each scenario, let me know,
> and I'll update the wiki.

  Yes, those instructions are perfect and indeed clearer.

>
> Finally, some explanation on how to verify the push you're about to do
> would be helpful.
>
> git push --dry-run is unhelpful to me - it doesn't show me what's about
> to go in.
> gitk can be confusing too if you don't understand all the UI bits.
> A simple command that would say 'here are the x commits you are about to
> push, plus their commit messages, plus their content' would be awesome.
> I'm sure it's in there somewhere.

  Before pushing, do a 'git-log origin/master..' (showing you all the
new commits that aren't in the remote master). If you see a 'Merge ...'
commit message, you should rebase before pushing.

>
> Lastly, it sounds to me that, if we don't want non-fast-forward-merge
> commits to happen, the tools should just not allow you to, no ? If we
> can make sure people don't commit wrongly indented code, why can't we
> make sure they don't do
> random-abracadabra-screwup-that-git-allows-but-we-don't-want ?

  Because we *sometime* require non-fast-forward merges. There's a good
article in today's (pay-for) LWN regarding git merge/rebase including
comments by Linus about this.

>
> Thomas
>
--
Edward Hervey <[hidden email]>


------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gstreamer-devel