emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Emacs rewrite in a maintainable language


From: Tassilo Horn
Subject: Re: Emacs rewrite in a maintainable language
Date: Wed, 14 Oct 2015 14:12:26 +0200
User-agent: Gnus/5.130014 (Ma Gnus v0.14) Emacs/25.0.50 (gnu/linux)

Oleh Krehel <address@hidden> writes:

>> OTOH, they reduce the helpfulness version control offers to new and
>> old developers.  I.e., it's good when "git blame" shows you the last
>> changes which actually changed the code, and switching "char const *"
>> to "const char *" or vice versa are no real changes.
>
> I've seen this excuse before. Doesn't it mean that there's a flaw in
> the "git blame" workflow rather than in the refactoring commits?

No.  That's a very effective workflow when it works, and it works quite
often with mature code.  For example, there are tons of lines in AUCTeX
which haven't been changed for more than a decade and `vc-annotate'
still suffices to get to know why some specific piece of code is like it
is.

> Besides, "git blame" provides a way to specify a range of revisions
> that you're interested in.

Yes, and we have `vc-region-history' which is fantastic.

> Personally, looking through the revision history in order to
> understand code is a last resort for me.  The code should be written
> and documented well enough to be understandable at the current
> revision.  This is further magnified by the fact that we release
> source tarballs, which should be self-containing.

Yes, obviously that's ideal.  Now of course the current version might
have introduced some new problem, so you can't always take it as a
trusty source of information.

>> In the same vein, it's of course good to have one consistent
>> indentation style, one consistent style of setting braces, one
>> consistent style of naming variables, etc.  But I'd suggest to clean
>> up the non-conforming parts only when you do significant changes in
>> that area anyhow.
>
> You mean:
>
>     /* TODO: change "char const *" to "const char *" when an opportunity 
> arises
>       to change foo */
>     void foo (char const *bar);

Yes, exactly that but without the TODO.  You just do it when you change
foo().

And don't get me wrong, I'm all for refactoring.  But IMHO switching
from one thing to a synonym of it or changing indentation from tabs to
spaces doesn't outweight the disadvantages of such "nothing really done"
commits.

> In the same vein, concerning the matter of one consistent indentation
> style, on occasion I've received complaints of producing too large
> whitespace diffs when changing Elisp functions.  On those occasions I
> had to twist myself into never using `indent-sexp' and inserting the
> appropriate amount of mixed tabs and spaces with "C-q". It wasn't fun.

For that reason, it's best to just go with the standard intentation
settings even when there's potential to improve them.  When I contribute
to your projects (and I like doing that, you do a great job!), I always
have to remember to turn off `aggressive-indent-mode' or later stage my
changes hunk-wise omitting hunks where only whitespace changes are
in. ;-)

Bye,
Tassilo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]