Re: [Tinycc-devel] Please, please at least notify the list when you make big changes

[David Mertens]

Hello grischka,

On Tue, Jul 25, 2017 at 6:47 AM, grischka <address@hidden> wrote:

David Mertens wrote:

Case in point:

I just tried to merge grischka's latest commit, described as "tccgen: nodata_wanted fix, default ONE_SOURCE, etc...". I have no idea why, but with the commit grischka decided to move a whole bunch of functionality out of libtcc.c's tcc_compile() and into tccgen.c's tccgen_compile(). Is this a good idea? Maybe? It makes tcc_compile read a little more clearly, though the real functionality is now buried under one more layer of abstraction. I don't know grischka's motivation because he never explained what he's doing or why.

IIRC "common setjmp clause" was the argument, as noted in the commit
comment.

And this is my point exactly. This work should have been isolated into its own commit, not mixed in with other things involving ONE_SOURCE and pseudo asm.

 

What I do know is that now two of my tests segfault, and I'll probably have to spend a couple of *hours* reverse-engineering how to track the symbols.

Hours of my life wasted for what seems like a frivolous change.

David, be sure that for a moment I thought about your extsyms when
doing exactly this change, but sorry, for one it had to be and two
it does not make sense to base decisions on possibly existing forks
elsewhere.

The community decided a while back to ignore considering forks when making design decisions. I am not asking to re-litigate that. I'm talking about good git practices.

 

See, once at some point in the past I suggested to you, rather than
a flag on the type to use a flag or pointer in the tokensym instead
which in my impression would behave much more orthogonal and friendly
wrt. maintenance.

And your answer was like "hm no, I like it please, because of the
smaller memory footprint,..." or so.  Okay man, right.  But then don't
complain with me because of merge pains.  All I was trying to say
is that "Hours of life" might be a valid argument when traded against
4 bytes more per symbol, and that some thoughts or maybe even hours
spent once on a effective defense strategy against upstream changes
might be a good investment at some point ... ;)

Your recollection of that design decision is stale, and it has nothing to do with my current issues merging your work into my fork. We are talking about making human-digestible commits to mob and talking about them on this list.

Since I complained about it, let's look at your commit 4b3c6. This commit accomplishes many distinct changes, at least nine based on the commit log. Each of these could have been uncoupled and placed into smaller commits. Now consider a person (like me) trying to read the commit patch, i.e. git log -p. The cognitive load of reading through an N-line commit diff scales super-linearly, so even if you split this commit into two smaller amalgamations, it would be easier for the human trying to read it. In fact, not only would it be easier for me to read through two separate commits, but I would also be able to skip or skim the commit that is not related to the setjmp clause. Good git practices stress separating distinct work into separate commits and this is a pristine example of why.

grischka, you are very smart. You know how to make minimal changes to solve problems. I am simply asking that you apply the same part of your brain to how you commit your work to tcc. It might take a little bit of practice, but I think you would be very good at it.

David