About commit style

About commit style

[Xue Fuqiao]

In admin/notes/commits:

   Each commit should correspond to a single change (whether spread over
   multiple files or not).  Do not mix different changes in the same
   commit (eg adding a feature in one file, fixing a bug in another
   should be two commits, not one).

Can I mix changes in _one_ file?  (E.g., adding a feature in one file,
fixing a bug in the same file)  When I want to add a feature in a file,
I often find bugs in the same file.  It is convenient and natural to fix
them without extra trouble, I think.

-- 
Xue Fuqiao
http://www.gnu.org/software/emacs/

Re: About commit style

[Eli Zaretskii]

> Date: Sat, 6 Apr 2013 16:13:08 +0800
> From: Xue Fuqiao <xfq.free@gmail.com>
> 
> In admin/notes/commits:
> 
>    Each commit should correspond to a single change (whether spread over
>    multiple files or not).  Do not mix different changes in the same
>    commit (eg adding a feature in one file, fixing a bug in another
>    should be two commits, not one).
> 
> Can I mix changes in _one_ file?  (E.g., adding a feature in one file,
> fixing a bug in the same file)

No, you cannot.

> When I want to add a feature in a file, I often find bugs in the
> same file.

Then fix the bugs (one by one, if they are different) in separate
commits, and then finally add the feature in one more commit.  You can
use separate branches for that, to reduce inconvenience if you find
the bugs when you are already half way into adding the feature.

The only fixes that can be mixed with other changes are whitespace
changes, incorrect formatting, etc.

The point to take here is that reverting a single revision does not
inadvertently introduces changes unrelated to the issue that is the
subject of that revision.  E.g., suppose your feature changes were
found to be incorrect or undesirable, and are reverted -- you don't
want that to re-introduce the unrelated bugs you fixed while working
on the feature, do you?

Re: About commit style

[Eli Zaretskii]

> Date: Sat, 06 Apr 2013 11:52:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> You can use separate branches for that, to reduce inconvenience if
> you find the bugs when you are already half way into adding the
> feature.

Or shelve your changes, fix the bug, commit it, then unshelve the
changes and continue.

Of (if you develop the feature on a separate branch), fix the bugs on
the trunk, in which case they don't affect your feature branch work.

Or cherry-pick versions from branch to trunk, when you commit.

I'm sure there are more ways of doing this, so that (a) bugfixes are
separate from unrelated features, and (b) your development workflow is
not disrupted or inconvenienced in any way.

Re: About commit style

[Xue Fuqiao]

On Sat, 06 Apr 2013 12:08:15 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Sat, 06 Apr 2013 11:52:44 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: emacs-devel@gnu.org
> > 
> > You can use separate branches for that, to reduce inconvenience if
> > you find the bugs when you are already half way into adding the
> > feature.
> 
> Or shelve your changes, fix the bug, commit it, then unshelve the
> changes and continue.
> 
> Of (if you develop the feature on a separate branch), fix the bugs on
> the trunk, in which case they don't affect your feature branch work.
> 
> Or cherry-pick versions from branch to trunk, when you commit.
> 
> I'm sure there are more ways of doing this, so that (a) bugfixes are
> separate from unrelated features, and (b) your development workflow is
> not disrupted or inconvenienced in any way.

I see, thank you.  I'll capture (using `org-capture') the bug if it
doesn't affect my feature branch work.  If not, I'll fix it first.

-- 
Xue Fuqiao
http://www.gnu.org/software/emacs/

Re: About commit style

[Stefan Monnier]

> Then fix the bugs (one by one, if they are different)

If they're trivial enough, it's perfectly OK to lump them together.


        Stefan

Re: About commit style

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 06 Apr 2013 09:10:43 -0400
> Cc: Xue Fuqiao <xfq.free@gmail.com>, emacs-devel@gnu.org
> 
> > Then fix the bugs (one by one, if they are different)
> 
> If they're trivial enough, it's perfectly OK to lump them together.

What are "trivial bugs" in this context?  Something beyond typos and
whitespace changes?

Re: About commit style

[Stefan Monnier]

>> > Then fix the bugs (one by one, if they are different)
>> If they're trivial enough, it's perfectly OK to lump them together.
> What are "trivial bugs" in this context?  Something beyond typos and
> whitespace changes?

Bugs which are due to using "just not quite the right code".
E.g. renaming a function/variable to use a proper prefix.


        Stefan

Re: About commit style

[Andreas Röhler]

Am 06.04.2013 17:32, schrieb Stefan Monnier:
>>>> Then fix the bugs (one by one, if they are different)
>>> If they're trivial enough, it's perfectly OK to lump them together.
>> What are "trivial bugs" in this context?  Something beyond typos and
>> whitespace changes?
>
> Bugs which are due to using "just not quite the right code".
> E.g. renaming a function/variable to use a proper prefix.
>
>
>          Stefan
>
>

Just for completeness it might pay to consider the case of a re-write too.
It must neither fix a bug nor introduce new features but might affect a bunch of lines.

During such a re-write a couple of steps deserve notice, while some of them will turn
out not so useful, being revoked before final commit.

Andreas

Re: About commit style

[Xue Fuqiao]

On Sat, 06 Apr 2013 11:52:44 +0300
Eli Zaretskii <eliz@gnu.org> wrote:

> The only fixes that can be mixed with other changes are whitespace
> changes, incorrect formatting, etc.

In etc/CONTRIBUTE:
---------------------------------------------------------------------
|** Do not make formatting changes.
|
|Making cosmetic formatting changes (indentation, etc) makes it harder
|to see what you have really changed.
---------------------------------------------------------------------

Can I make changes like this:

* Reindent the file using the default indentation parameters.

* Change some code from putting close-parentheses on lines by themselves
  to Lisp style.  (Although this almost never happened.)

* Change the comments to satisfy (info "(elisp) Comment Tips").

-- 
Xue Fuqiao
http://www.gnu.org/software/emacs/

Re: About commit style

[Eli Zaretskii]

> Date: Sun, 7 Apr 2013 14:17:06 +0800
> From: Xue Fuqiao <xfq.free@gmail.com>
> Cc: emacs-devel@gnu.org
> 
> On Sat, 06 Apr 2013 11:52:44 +0300
> Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > The only fixes that can be mixed with other changes are whitespace
> > changes, incorrect formatting, etc.
> 
> In etc/CONTRIBUTE:
> ---------------------------------------------------------------------
> |** Do not make formatting changes.

It means "do not make formatting changes ALONE".  They are OK as part
of another, non-trivial change.

Re: About commit style

[Stefan Monnier]

> * Reindent the file using the default indentation parameters.

No.  You can only fix local misindentation.

Any "bulk" cosmetic changes are annoying because they are sure to bump into
someone's local changes, creating spurious conflicts.

> * Change some code from putting close-parentheses on lines by themselves
>   to Lisp style.  (Although this almost never happened.)

Same here: doing it somewhere where you're already making other changes
is fine, but doing it systematically is not.

> * Change the comments to satisfy (info "(elisp) Comment Tips").

Idem.


        Stefan

Re: About commit style

[Xue Fuqiao]

On Sun, 07 Apr 2013 11:48:47 -0400
Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> > * Reindent the file using the default indentation parameters.
> 
> No.  You can only fix local misindentation.
> 
> Any "bulk" cosmetic changes are annoying because they are sure to bump into
> someone's local changes, creating spurious conflicts.
> 
> > * Change some code from putting close-parentheses on lines by themselves
> >   to Lisp style.  (Although this almost never happened.)
> 
> Same here: doing it somewhere where you're already making other changes
> is fine, but doing it systematically is not.
> 
> > * Change the comments to satisfy (info "(elisp) Comment Tips").
> 
> Idem.

I see, thanks.

-- 
Xue Fuqiao
http://www.gnu.org/software/emacs/