Git best practice

118 views
Skip to first unread message

Simon King

unread,
Jun 25, 2015, 5:07:53 PM6/25/15
to sage-...@googlegroups.com
Hi!

Trac ticket #18758 depends on #18756. The former has no branch yet, the
latter has a branch, but it is not clear to me whether the last two
commits will actually make it into the final version (it is a proof of
concept, but one may argue if it is a good idea).

So, the question is: What should be the starting point of a branch for
#18758? The branch of #18756 ("the last two commits WILL stay, but they
may be reverted by a future commit"), or the branch of #18756 without
the last two commits ("IF the last two commits will stay, we can merge
them into #18758 later")?

Best regards,
Simon


Travis Scrimshaw

unread,
Jun 25, 2015, 7:26:32 PM6/25/15
to sage-...@googlegroups.com
Hey Simon,
   You could just make 2 branches, one for each of the commits and cherry-pick commits between the two branches (or just keep a reference to the commits you don't want).

Best,
Travis

Simon King

unread,
Jun 25, 2015, 7:52:08 PM6/25/15
to sage-...@googlegroups.com
Hi Travis,

On 2015-06-25, Travis Scrimshaw <tsc...@ucdavis.edu> wrote:
> You could just make 2 branches, one for each of the commits and
> cherry-pick commits between the two branches (or just keep a reference to
> the commits you don't want).

I know that I can do this. I want to know: What should I push to trac?

Which of the two approaches ("Keep the critical commits in and perhaps
revert them in a later commit" or "Leave the critical commits out and
perhaps insert them later") has the least probability of confusing a
referee or upsetting the release manager?

Best regards,
Simon


Jeroen Demeyer

unread,
Jun 26, 2015, 2:43:48 AM6/26/15
to sage-...@googlegroups.com
On 2015-06-26 01:51, Simon King wrote:
> Which of the two approaches ("Keep the critical commits in and perhaps
> revert them in a later commit" or "Leave the critical commits out and
> perhaps insert them later") has the least probability of confusing a
> referee or upsetting the release manager?

Do you expect both tickets to get reviewed at the same time? If not,
then it doesn't matter what you do. If you do want both tickets reviewed
at the same time, ask your potential reviewer what he wants.

And the release manager probably doesn't care much, it's only the result
that matters.

Volker Braun

unread,
Jun 26, 2015, 3:40:44 AM6/26/15
to sage-...@googlegroups.com
The best solution is probably to implement your proof of concept in an altogether separate ticket.

If in doubt always use the minimal number of commits; There is nothing to be gained by extra stuff in the history that you revert later.

Simon King

unread,
Jun 26, 2015, 6:45:30 AM6/26/15
to sage-...@googlegroups.com
Hi Volker,

On 2015-06-26, Volker Braun <vbrau...@gmail.com> wrote:
> The best solution is probably to implement your proof of concept in an
> altogether separate ticket.

No, it is a proof of concept of one possible interpreation of the
ticket's topic, and thus I wouldn't open a new ticket for it. The topic
is about the possibility to implement _get_action_ as a parent method
of the category. And the open question is whether Parent.get_action
(without underscore) should simply look at the first "categorical"
_get_action_ method, or should go up the category hierarchy until the
corresponding _get_action_ returns something non-None.

> If in doubt always use the minimal number of commits; There is nothing to
> be gained by extra stuff in the history that you revert later.

If I recall correctly, I found severeal git introductions recommending
to commit often and early. And how should one possibly show the code to
a ticket's referee if not by a commit?

Best regards,
Simon


Volker Braun

unread,
Jun 26, 2015, 7:52:03 AM6/26/15
to sage-...@googlegroups.com
On Friday, June 26, 2015 at 12:45:30 PM UTC+2, Simon King wrote:
> If in doubt always use the minimal number of commits; There is nothing to
> be gained by extra stuff in the history that you revert later.

If I recall correctly, I found severeal git introductions recommending
to commit often and early. And how should one possibly show the code to
a ticket's referee if not by a commit? 

What I meant was: You should commit often, but you shouldn't import said commits into other branches if you can avoid it.

Simon King

unread,
Jun 26, 2015, 9:11:32 AM6/26/15
to sage-...@googlegroups.com
Hi Volker,

On 2015-06-26, Volker Braun <vbrau...@gmail.com> wrote:
> What I meant was: You should commit often, but you shouldn't import said
> commits into other branches if you can avoid it.

OK, that makes sense. Thank you.

Cheers,
Simon

Travis Scrimshaw

unread,
Jun 26, 2015, 11:46:56 AM6/26/15
to sage-...@googlegroups.com
Hey Simon,
   Ah, sorry. From all the different git workflows I've seen and read, it is better to push your comments and revert them in later commits because it shows a history of things you tried (and possibly pull code from).

Best,
Travis

Travis Scrimshaw

unread,
Jun 26, 2015, 11:49:59 AM6/26/15
to sage-...@googlegroups.com

If I recall correctly, I found severeal git introductions recommending
to commit often and early. And how should one possibly show the code to
a ticket's referee if not by a commit? 

What I meant was: You should commit often, but you shouldn't import said commits into other branches if you can avoid it.

I only mildly agree with this statement. I would say, "you shouldn't import unrelated commits into other branches if you can avoid it."

Best,
Travis

Nathann Cohen

unread,
Jun 26, 2015, 3:11:59 PM6/26/15
to sage-...@googlegroups.com
   Ah, sorry. From all the different git workflows I've seen and read, it is better to push your comments and revert them in later commits because it shows a history of things you tried (and possibly pull code from).

Once in a branch somebody moved a lot of code from one place to another. Another commit reverted the change, and moved everything back. In this situation it was better to merge the two commits which undo each other: by having two commits instead, the results of "git blame" changes (the lines were last modified by the 'undo' commit).

Nathann 

Travis Scrimshaw

unread,
Jun 27, 2015, 1:29:09 AM6/27/15
to sage-...@googlegroups.com
In that situation (which I consider as a special case), one should instead create a new branch based off the point right before the move happened. Don't change history!

Travis

Nathann Cohen

unread,
Jun 27, 2015, 1:32:13 AM6/27/15
to Sage devel
> In that situation (which I consider as a special case), one should instead
> create a new branch based off the point right before the move happened.

Works for me, since the final branch is the same.

> Don't change history!

You would call it "rewriting history" if, after the merge, you push
the new branch where the previous one was. You would call it "creating
a new branch" if you push it somewhere else.

Nathann

Simon King

unread,
Jun 27, 2015, 2:14:58 AM6/27/15
to sage-...@googlegroups.com
Hi Travis,

On 2015-06-27, Travis Scrimshaw <tsc...@ucdavis.edu> wrote:
> In that situation (which I consider as a special case), one should instead
> create a new branch based off the point right before the move happened.

Actually, after posting, I realised that in the special situation I need
the commits (or at least part of it).

> Don't change history!

I.e., you suggest to keep the two commits and potentially revert them by
an additional commit (because everything that was ever accidentally posted
on trac belongs to SageMath's history)?

Anyway, in the special situation it is needed to keep them (or at least part
of them) anyway.

Best regards,
Simon

Reply all
Reply to author
Forward
0 new messages