Re: [PATCH 1/2] gitview: Use new-style classes

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] gitview: Use new-style classes

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:13

Michael Ellerman [off-list ref] writes:
Signed-off-by: Michael Ellerman <redacted>
Aneesh, this one is in your bailiwick.

Michael, it would have been nicer to have a real log message,
not just the Subject line, in your message.  The Subject line
says what the patch does in what subarea of the git.git tree,
which is very appropriate, but that can be seen from the diff
even if you did not say so.  The log message (before your
Signed-off-by line) is where you describe _why_ you would think
it is a good change.  "What" and "how" are usually evident in
well written code without much explanation, but "why" is often
more useful when reviewing the patch, and reading and studying
the history of the code.

I would have written something like:

	This changes the Commit class to use new-style class,
	which has been available since Python 2.2 (Dec 2001).
	This is a necessary step in order to use __slots__
	declaration so that we can reduce the memory footprint
	with my next patch.

I have no strong preference on this change myself.  "Classes
that derive from type" are relatively recent invention but I
would personally feel that Python 2.2 is ancient enough that
using it would not pose any practical portability issues, but
what would I know...

The __slots__ patch depends on this, and that one has a
measurable memory footprint improvement, so I would say we
should take it.

Re: [PATCH 1/2] gitview: Use new-style classes

From: Michael Ellerman <hidden>
Date: 2016-06-15 22:43:14

On Sat, 2007-06-02 at 02:38 -0700, Junio C Hamano wrote:
Michael Ellerman [off-list ref] writes:
quoted
Signed-off-by: Michael Ellerman <redacted>
Aneesh, this one is in your bailiwick.

Michael, it would have been nicer to have a real log message,
not just the Subject line, in your message.  The Subject line
says what the patch does in what subarea of the git.git tree,
which is very appropriate, but that can be seen from the diff
even if you did not say so.  The log message (before your
Signed-off-by line) is where you describe _why_ you would think
it is a good change.  "What" and "how" are usually evident in
well written code without much explanation, but "why" is often
more useful when reviewing the patch, and reading and studying
the history of the code.

I would have written something like:

	This changes the Commit class to use new-style class,
	which has been available since Python 2.2 (Dec 2001).
	This is a necessary step in order to use __slots__
	declaration so that we can reduce the memory footprint
	with my next patch.

I have no strong preference on this change myself.  "Classes
that derive from type" are relatively recent invention but I
would personally feel that Python 2.2 is ancient enough that
using it would not pose any practical portability issues, but
what would I know...
Hi Junio,

Why write my own changelog when you do such a good job! ;)

But seriously, sorry about that. It was a case of me hacking on my local
git tree, not thinking anything would come of it, and then when I saw
the memory reduction from using __slots__ I decided to send off what I'd
done. I should have reset and recommitted with a decent changelog.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help