From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johan Herland <johan@herland.net>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: RFC: "negative" dirstat
Date: Mon, 18 Apr 2011 14:46:24 -0700 [thread overview]
Message-ID: <BANLkTimEUSb9Na7Z+tnmytsqeeKLkQb9xA@mail.gmail.com> (raw)
In-Reply-To: <7vzknn6y7y.fsf@alter.siamese.dyndns.org>
On Mon, Apr 18, 2011 at 2:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Are these removal really "remove old cruft, replace with a better version
> which does not have much common to removed stuff", or are they more like
> "remove N duplicated similar copies of old cruft, refactoring them
> properly and the result is used by N callsites"?
Right now, there not so much of either.
The _hope_ is that there will be "remove <n> different implementations
of some gpio driver, and replace them with a couple of generic ones
and some setup code".
So no, it wouldn't necessarily be about code movement at all, but
about totally different code.
> The second reason you gave in an earlier discussion why dirstat uses the
> damage assessor code was to disregard code movements.
No, that only works within a file, which sometimes is common when you
have to re-order functions because or some re-organization.
See for example commit 9d412a43c3b2 ("vfs: split off vfsmount-related
parts of vfs_kern_mount()") in the kernel for an example of why line
ordering shouldn't necessarily count against damage.
But what I'm talking about is really different code, not
re-organization of existing one.
> I guess what it boils down to is what you are trying to measure as the
> "goodness" value of a change.
Yes.
> Adding a lot of Documentation may be good,
> adding a lot of "subarchs that do not deserve to be" may be bad, and
> moving common logic from one existing subarch to a common file (which
> counts towards "literal-added" in that new common file, at the same time
> counting towards deletion, i.e. "size - copied", from the original) and
> reusing it in a new subarch by simply calling that common infrastructure
> is a very good thing.
Yes. If I see a lot of lines added in Documentation/, I really don't
mind at all. And if I see a diffstat that says something like
1057 lines added, 2901 lines deleted
I'm extremely happy. It's very different from one that would say
2958 lines added, 0 lines deleted
even though --dirstat might consider the two equivalent right now.
> At least, if you count literal-added vs src-copied
> across the files within the subarch, instead of doing it per-file, you
> would be able to detect the "moving" part more accurately.
Yes. However, "moving" is in many ways still not as interesting as
"actually removed".
Moving, on it's own, is a pretty neutral operation. So I really don't
want to concentrate on the moving part. It's really about "some
operations actually clean up code and remove code in the process".
Linus
next prev parent reply other threads:[~2011-04-18 21:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-18 21:02 RFC: "negative" dirstat Linus Torvalds
2011-04-18 21:33 ` Junio C Hamano
2011-04-18 21:46 ` Linus Torvalds [this message]
2011-04-19 4:51 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BANLkTimEUSb9Na7Z+tnmytsqeeKLkQb9xA@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).