Hi Gordon,
first of all, I am moving this to discuss@
On 15.05.2012 18:06, Gordon Ross wrote:
In a recent code review discussion, we found ourselves in a
discussion
of the general dislike for changes that touch a lot of places in our
code.
I've been meaning to write something about this for a while, so here
is my attempt to explain.
In an open source project, our most precious commodity is the time of
our volunteers. (A fact I was reminded of by this excellent talk
http://video.google.com/videoplay?docid=-4216011961522818645 by Ben
Collins-Sussman and Brian Fitzpatrick).
Any change you make generally costs the project the time of some
reviewers (our precious commodity).
That time is proportional to the number of places you touch.
Further, you should assume that any changes you make need to be
merged
by someone working on a source base derived from the one you're
changing, and often several people people need to spend their time on
this. The more places you change, the more "cost" this forces onto
your "merge victims".
What I take from this is the guidance: Make it worth their while.
If you're going to make a change in a way that touches a lot of
places, first try to find a way to do it that does not require
changing a lot of places. Then if you can't, only make the change if
doing so gives the reviewer and merge victims some non-trivial "pay
back" in exchange for their efforts.
This is why I generally dislike sweeping "cleanup" changes. They
tend
to touch a lot of places, and not give me much in return for my
review
and/or merge efforts. On the other hand, if you're adding a major
new
feature, I'll likely forgive some cleanup in and around what you
touch.
I am well-aware that I am doing a lot of small and big changes with
minimal impact on the functionality, e.g. with lint 12.3 changes I
touched frequently parts of gate. This fulfill what you are writing
about.
And yes, it consumed a lot of time for reviewers. Time which could be
spent by something else, like the next ZFS.
I hope I tried to push for reviews gently. If not then I am sorry.
So I take your e-mail as something which is against my work here
and I would like to explain my position.
I have reasons for such changes, even if they look unnecessary.
Based on personal experience in several Solaris gates.
If the rules are followed less strictly and/or code styles diverge
and/or code is not updated over years then it is leading to
generally worse code in these ways:
1) new people starting to look at the source code are scared from
obvious problems of the source code and "mess" in it
2) analysis of the code is taking longer because you have to think
as many very different developers
3) people are afraid to touch obvious problems because "they
have to exist for some less-obvious reasons"
And I believe we should use as much as possible from tools we can
to make our code better in ways of functionality and also readability.
Yes, lint 12.3 changes brought some strange things in (cases with
unreachable empty commands) but for such cases I tried to do more,
e.g. by removal of code duplicities and/or removal of the dead code.
And this I consider as benefit to the code base.
If I did not fix all of them then I could miss those few real.
You could see from final changes that I missed some during my work
on it.
Throwing in tons of new code with super-uber-cool functionality look
nice in short term. But if you need to look at such code 10 years later
you are scared.
I will continue with fixing corner cases and doing clean up of the
code as long as I will find reviewers and advocates taking some
value from it. If members of dev-council and/or advocates will say
"Stop, we are not interested" then I have no other options than
to respect it.
I am not paid by any party to work on Illumos and I am not using
it for anything commercial, only for home systems. So my contribs
are usually very small, limited to time I have and want to spend
and I can stop with them and survive :-)
Yes, such changes can bring headache to backporters to private gates.
As ex-RPE I know even these effects.
Additionally, experience from other open source communities is that
some big contributors are starting on these clean ups. It would be
bad idea to stop them and expect that all new people will come
just with great things.
Best regards,
Milan
P.S.: I am sorry for late reply, my mail server selected to queue
few e-mails for 2 days for me and they arrive finally now.
-------------------------------------------
illumos-discuss
Archives: https://www.listbox.com/member/archive/182180/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182180/21175430-2e6923be
Modify Your Subscription:
https://www.listbox.com/member/?member_id=21175430&id_secret=21175430-6a77cda4
Powered by Listbox: http://www.listbox.com
