You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@archiva.apache.org by Joakim Erdfelt <jo...@erdfelt.com> on 2007/10/10 21:25:01 UTC
Monster Commit Breakdown - svn commit: r583412 [1/8]
This is an attempt to breakdown the monster commit to help people not
fear it.
(Disclaimer: The views / opinions / rants herein are from a mad swede,
myself, and do not reflect the views of the other devs
and commitors in archiva. Do not take the statements made
within this email as canon of how things operate within
archiva. I have been merely asked to explain the monster
commit as it is out of form and considered harmful to
the project.)
Was what I did wrong? Probably.
Was there a more appropriate way? Probably.
Was this a surprise commit? No.
Did this destablize the codebase? No.
Was the size of this commit excessive? No. (1)
Could I have split up this into different commits? Probably. (2)
Why did I do this? Time. (3)
Will I change? Damn straight!
Technical Breakdown of commit:
Label Size : Self : Overall
----------------------------------------------:------:--------
Totals (overall):
Change Blocks .......................... 362 : :
Property Changes ........................ 96 : :
Count Entries (Total) .................. 131 : :
Count Entries Added ..................... 32 : 24% :
Count Entries Removed ................... 11 : 8% :
Count Entries Modified .................. 87 : 66% :
Count Entries Property Change Only ....... 1 : 1% :
Lines (Total) ........................ 19225 : :
Lines of Diff Overhead ................ 3994 : 21% :
Lines Added (Total) .................. 13126 : 68% :
Lines Added from New Entries ......... 11519 : 60% :
Lines Added from Modified Entries ..... 1607 : 8% :
Lines Removed (Total) ................. 2105 : 11% :
Lines Removed from Deleted Entries ..... 754 : 4% :
Lines Removed from Modified Entries ... 1351 : 7% :
Lines Modified Delta (Added - Removed) . 256 : :
Lines of Comment Changes .............. 1716 : 9% :
Totals (src/main):
Change Blocks .......................... 156 : : 43%
Property Changes ........................ 54 : : 56%
Count Entries (Total) ................... 62 : : 47%
Count Entries Added ..................... 18 : 29% : 14%
Count Entries Removed .................... 3 : 5% : 2%
Count Entries Modified .................. 41 : 66% : 31%
Count Entries Property Change Only ....... 0 : 0% : 0%
Lines (Total) ......................... 6680 : : 35%
Lines of Diff Overhead ................ 1818 : 27% : 9%
Lines Added (Total) ................... 3815 : 57% : 20%
Lines Added from New Entries .......... 2884 : 43% : 15%
Lines Added from Modified Entries ...... 931 : 14% : 5%
Lines Removed (Total) ................. 1047 : 16% : 5%
Lines Removed from Deleted Entries ..... 210 : 3% : 1%
Lines Removed from Modified Entries .... 837 : 13% : 4%
Lines Modified Delta (Added - Removed) .. 94 : :
Lines of Comment Changes .............. 1015 : 15% : 5%
Totals (src/test):
Change Blocks .......................... 206 : : 57%
Property Changes ........................ 42 : : 44%
Count Entries (Total) ................... 69 : : 53%
Count Entries Added ..................... 14 : 20% : 11%
Count Entries Removed .................... 8 : 12% : 6%
Count Entries Modified .................. 46 : 67% : 35%
Count Entries Property Change Only ....... 1 : 1% : 1%
Lines (Total) ........................ 12545 : : 65%
Lines of Diff Overhead ................ 2176 : 17% : 11%
Lines Added (Total) ................... 9311 : 74% : 48%
Lines Added from New Entries .......... 8635 : 69% : 45%
Lines Added from Modified Entries ...... 676 : 5% : 4%
Lines Removed (Total) ................. 1058 : 8% : 6%
Lines Removed from Deleted Entries ..... 544 : 4% : 3%
Lines Removed from Modified Entries .... 514 : 4% : 3%
Lines Modified Delta (Added - Removed) . 162 : :
Lines of Comment Changes ............... 701 : 6% : 4%
(The code for this output is available on my personal svn space)
Interesting Features of the Technical Breakdown:
* There was only a delta of 256 lines changed in existing files.
* There is an almost 2::1 ratio of test to main code.
* 20% of the diff is overhead.
* 9% of the diff is comment changes. (thats alot)
* The majority of the new files (29%) arrived into src/main directories.
* The majority of the diff (57%) occurs in the src/test directories.
* The majority of new content/lines (74%) arrived into src/test
directories.
(1) I submit that this is not an excessive commit, as seen from the
facts above.
The Reasons For It:
First: THIS IS NOT A MAVEN 1 LEGACY ONLY PROBLEM as some people have
suggested. The way layouts were used within archiva prior to
this commit made the entire core unreliable, inconsistent, and
subject to failure in real world scenarios.
As we get more and more users of Archiva for their day to day projects,
we see more and more examples of legitimate usage of Archiva where the
most basic functions fail due to a one layout assumption or another.
The jist of the commit is to eliminate the possibility of failure by
removing the ability to take shortcuts in the code.
There was also plenty of uncoding done here to join together the
various implementations of finding versions, or related files,
or updating metadata.
(2) I could have split this commit up into separate pieces, but decided
that getting this code into place was more important that spending the
next several days carefully crafting the optimum separation of commits
to perform that wouldn't break the build, or have a dependency on another
commit coming up. This was my decision, I was not coerced into it, I
weighed the options (3) and felt that getting a version 1.0 out sooner
rather than later was more important.
I would love to see Archiva 1.0 final out before ApacheCon US
(starting on November 10th, 2007)
This code is a bug fix for current open and assigned jiras, it is not
a rearchitecting, or work against future issues, or even an attempt at
code perfection.
Archiva is a web application, and as such, will have a different way
of handling changes to the codebase, it is not subject to the the same
stringent commit and change controls in place for maven, or java libs,
or components that are either released, shared, or in common use across
many users or other applications. This is the best time to make these
kind of changes, before the release, before the web services.
I acknowledge that I could have branched this, which given the flak i'm
getting for this commit, I probably should have. But I stand by this
change as a means to get the product stable in a faster fashion.
I apologize for scaring the crap out of the other devs.
I apologize for for not making a branch first.
I apologize for not splitting up the commit.
I apologize for stomping across other peoples active work.
In the future I promise to be more clear about the scope of changes
that need to be done.
I promise to better manage my commit sizes and scope in the future.
Hopefully, this has addressed any concerns about this monster commit.
If not, please, by all means, SAY SOMETHING. ;-)
In the end, I'm sure every developer and commitor for Archiva will
appreciate this change for what it is.
--
- Joakim Erdfelt
joakim@erdfelt.com
Open Source Software (OSS) Developer
Re: Monster Commit Breakdown - svn commit: r583412 [1/8]
Posted by Wendy Smoak <ws...@gmail.com>.
Brett is a difficult act to follow on this one, and I agree with
everything he said, so I don't have too much to add.
On 10/10/07, Joakim Erdfelt <jo...@erdfelt.com> wrote:
{snip justification}
> (1) I submit that this is not an excessive commit, as seen from the
> facts above.
This misses an important point. The statistics don't matter. The
community does. Apache projects are communities, not codebases. I
think of "Archiva" as a group of people collaborating on a solution to
a common problem, rather than as a webapp. That changes what I see as
the 'right' decision in a given context.
I'm sure everyone has heard the expression "community is more
important than code". Doing something to get closer to a release, at
the expense of the other contributors understanding and coming to a
consensus on the necessary changes, is not progress.
No, the commit wasn't a surprise, because it was preceded by a
proposal that seemed to me more like an announcement of intent. That
this particular one spanned eight email messages is not the point.
I'd be just as concerned about a single message in similar
circumstances.
> Archiva is a web application, and as such, will have a different way
> of handling changes to the codebase, it is not subject to the the same
> stringent commit and change controls in place for maven, or java libs,
> or components that are either released, shared, or in common use across
> many users or other applications.
Small, reversible changes associated with a JIRA issue are the way to
go. There is no reason web application development needs to involve
complex commits with multiple changes, and I don't buy the argument
that webapps should be exempt from disciplined development practices.
All in all, though, I'm looking forward to a 1.0, whether it comes
before or after ApacheCon US. :)
--
Wendy
Re: Monster Commit Breakdown - svn commit: r583412 [1/8]
Posted by Brett Porter <br...@apache.org>.
Hi Joakim,
Thanks for discussing this. I don't want to dwell on it since I think
we are in agreement that the change is the right technical direction
but that it should not have been done in such a "singular" fashion.
However, there are a couple of things I don't quite agree with and
wanted to pick up on. I'm not sure if we differ in opinion or if I'm
just reading through jetlagged eyes :)
Also, though this thread is now largely unrelated to the change
itself - please reply to my other mail which addresses specific
technical issues that are outstanding. I'm particularly concerned
that the purge feature is no longer working (and in a way that
indicates version parsing may be broken entirely based on the error I
got).
On 10/10/2007, at 9:25 PM, Joakim Erdfelt wrote:
> Technical Breakdown of commit:
This is pretty consistent with what I observed from scanning it, but
I think that supports the whole point in my other mail - unrelated
changes shouldn't go together. I just wanted to stress my objection
was not to the size of the commit, but the complexity.
> First: THIS IS NOT A MAVEN 1 LEGACY ONLY PROBLEM as some people have
> suggested. The way layouts were used within archiva prior to
> this commit made the entire core unreliable, inconsistent, and
> subject to failure in real world scenarios.
Sorry, but to me that wasn't clear from the proposal - all the
discussion centred around those particular problems. We need to work
on better describing the problems being addressed by changes.
>
> As we get more and more users of Archiva for their day to day
> projects,
> we see more and more examples of legitimate usage of Archiva where
> the
> most basic functions fail due to a one layout assumption or another.
Just IMO, but I think this is overstating the problem a little :)
>
> (2) I could have split this commit up into separate pieces, but
> decided
> that getting this code into place was more important that spending
> the
> next several days carefully crafting the optimum separation of
> commits
> to perform that wouldn't break the build, or have a dependency on
> another
> commit coming up. This was my decision, I was not coerced into it, I
> weighed the options (3) and felt that getting a version 1.0 out
> sooner
> rather than later was more important.
> I would love to see Archiva 1.0 final out before ApacheCon US
> (starting on November 10th, 2007)
I don't agree here. Crafting small changesets should not be more work
than large ones - it's simply a matter of discipline. And we cannot
cut corners to speed a release - we've seen that it causes the
opposite effect already.
>
> This code is a bug fix for current open and assigned jiras, it is not
> a rearchitecting, or work against future issues, or even an
> attempt at
> code perfection.
The end result was that it did re-architect something, based on a
proposal that was still being discussed. I think we need to be more
critical of the need for that (see below).
>
> Archiva is a web application, and as such, will have a different way
> of handling changes to the codebase, it is not subject to the the
> same
> stringent commit and change controls in place for maven, or java
> libs,
> or components that are either released, shared, or in common use
> across
> many users or other applications. This is the best time to make
> these
> kind of changes, before the release, before the web services.
I don't agree with this. Good development practices should apply
everywhere. Yes, it's easier to get away with making sweeping changes
now, but that doesn't make it the right choice.
I realise I'm oversimplifying, but I think it's important to agree on
the ideals and make concessions only where necessary.
>
> I acknowledge that I could have branched this, which given the
> flak i'm
> getting for this commit, I probably should have. But I stand by this
> change as a means to get the product stable in a faster fashion.
Stable? "You keep using that word. I do not think it means what you
think it means." :)
It is stable when it is "not likely to change or fail". A known bug
is not a stability problem. Within the scope I use it, it runs 24x7
on my machine without failing. I think it's stable.
Large changes decrease (development) stability by definition.
If we really want to seek stability, we need to be making small,
testable changes towards the 1.0 release from here on out.
>
> I apologize for scaring the crap out of the other devs.
> I apologize for for not making a branch first.
> I apologize for not splitting up the commit.
> I apologize for stomping across other peoples active work.
>
> In the future I promise to be more clear about the scope of changes
> that need to be done.
>
> I promise to better manage my commit sizes and scope in the future.
Thanks, much appreciated!
- Brett
--
Brett Porter - brett@apache.org
Blog: http://www.devzuz.org/blogs/bporter/
Re: Monster Commit Breakdown - svn commit: r583412 [1/8]
Posted by Maria Odea Ching <oc...@exist.com>.
Thanks for this email Joakim :) It helped people to better understand
all the recent changes in the codebase. I admit I have also panicked
when I saw some of the classes I was using in my codes (that I still
haven't committed yet) were removed, but after reading the past threads
plus this email I think I know (at least) where to find or what to use
in place for those classes.
Anyway, as what has already been mentioned by Brett and Wendy.. it
would've been better to have the changes committed piece by piece
instead of a giant commit as a lot of other developers/contributors
would get affected especially since we're nearing the 1.0 release. I
admit, I am also guilty of doing this at times and maybe we could all
learn from this so-called 'monster commit panic' to practice committing
our changes piece by piece :-)
-Deng
Joakim Erdfelt wrote:
> This is an attempt to breakdown the monster commit to help people not
> fear it.
>
> (Disclaimer: The views / opinions / rants herein are from a mad swede,
> myself, and do not reflect the views of the other devs
> and commitors in archiva. Do not take the statements made
> within this email as canon of how things operate within
> archiva. I have been merely asked to explain the monster
> commit as it is out of form and considered harmful to
> the project.)
>
> Was what I did wrong? Probably.
> Was there a more appropriate way? Probably.
> Was this a surprise commit? No.
> Did this destablize the codebase? No.
> Was the size of this commit excessive? No. (1)
> Could I have split up this into different commits? Probably. (2)
> Why did I do this? Time. (3)
> Will I change? Damn straight!
>
> Technical Breakdown of commit:
>
> Label Size : Self : Overall
> ----------------------------------------------:------:--------
>
> Totals (overall):
> Change Blocks .......................... 362 : : Property
> Changes ........................ 96 : : Count Entries (Total)
> .................. 131 : : Count Entries Added
> ..................... 32 : 24% : Count Entries Removed
> ................... 11 : 8% : Count Entries Modified
> .................. 87 : 66% : Count Entries Property Change Only
> ....... 1 : 1% : Lines (Total) ........................ 19225
> : : Lines of Diff Overhead ................ 3994 : 21% :
> Lines Added (Total) .................. 13126 : 68% : Lines Added
> from New Entries ......... 11519 : 60% : Lines Added from
> Modified Entries ..... 1607 : 8% : Lines Removed (Total)
> ................. 2105 : 11% : Lines Removed from Deleted Entries
> ..... 754 : 4% : Lines Removed from Modified Entries ... 1351
> : 7% : Lines Modified Delta (Added - Removed) . 256 : :
> Lines of Comment Changes .............. 1716 : 9% :
> Totals (src/main):
> Change Blocks .......................... 156 : : 43%
> Property Changes ........................ 54 : : 56%
> Count Entries (Total) ................... 62 : : 47%
> Count Entries Added ..................... 18 : 29% : 14%
> Count Entries Removed .................... 3 : 5% : 2%
> Count Entries Modified .................. 41 : 66% : 31%
> Count Entries Property Change Only ....... 0 : 0% : 0%
> Lines (Total) ......................... 6680 : : 35%
> Lines of Diff Overhead ................ 1818 : 27% : 9%
> Lines Added (Total) ................... 3815 : 57% : 20%
> Lines Added from New Entries .......... 2884 : 43% : 15%
> Lines Added from Modified Entries ...... 931 : 14% : 5%
> Lines Removed (Total) ................. 1047 : 16% : 5%
> Lines Removed from Deleted Entries ..... 210 : 3% : 1%
> Lines Removed from Modified Entries .... 837 : 13% : 4%
> Lines Modified Delta (Added - Removed) .. 94 : : Lines of
> Comment Changes .............. 1015 : 15% : 5%
>
> Totals (src/test):
> Change Blocks .......................... 206 : : 57%
> Property Changes ........................ 42 : : 44%
> Count Entries (Total) ................... 69 : : 53%
> Count Entries Added ..................... 14 : 20% : 11%
> Count Entries Removed .................... 8 : 12% : 6%
> Count Entries Modified .................. 46 : 67% : 35%
> Count Entries Property Change Only ....... 1 : 1% : 1%
> Lines (Total) ........................ 12545 : : 65%
> Lines of Diff Overhead ................ 2176 : 17% : 11%
> Lines Added (Total) ................... 9311 : 74% : 48%
> Lines Added from New Entries .......... 8635 : 69% : 45%
> Lines Added from Modified Entries ...... 676 : 5% : 4%
> Lines Removed (Total) ................. 1058 : 8% : 6%
> Lines Removed from Deleted Entries ..... 544 : 4% : 3%
> Lines Removed from Modified Entries .... 514 : 4% : 3%
> Lines Modified Delta (Added - Removed) . 162 : : Lines of
> Comment Changes ............... 701 : 6% : 4%
>
> (The code for this output is available on my personal svn space)
>
> Interesting Features of the Technical Breakdown:
>
> * There was only a delta of 256 lines changed in existing files.
> * There is an almost 2::1 ratio of test to main code.
> * 20% of the diff is overhead.
> * 9% of the diff is comment changes. (thats alot)
> * The majority of the new files (29%) arrived into src/main directories.
> * The majority of the diff (57%) occurs in the src/test directories.
> * The majority of new content/lines (74%) arrived into src/test
> directories.
>
> (1) I submit that this is not an excessive commit, as seen from the
> facts above.
>
> The Reasons For It:
>
> First: THIS IS NOT A MAVEN 1 LEGACY ONLY PROBLEM as some people have
> suggested. The way layouts were used within archiva prior to
> this commit made the entire core unreliable, inconsistent, and
> subject to failure in real world scenarios.
>
> As we get more and more users of Archiva for their day to day projects,
> we see more and more examples of legitimate usage of Archiva where the
> most basic functions fail due to a one layout assumption or another.
>
> The jist of the commit is to eliminate the possibility of failure by
> removing the ability to take shortcuts in the code.
>
> There was also plenty of uncoding done here to join together the
> various implementations of finding versions, or related files,
> or updating metadata.
>
> (2) I could have split this commit up into separate pieces, but decided
> that getting this code into place was more important that spending the
> next several days carefully crafting the optimum separation of commits
> to perform that wouldn't break the build, or have a dependency on
> another
> commit coming up. This was my decision, I was not coerced into it, I
> weighed the options (3) and felt that getting a version 1.0 out sooner
> rather than later was more important.
> I would love to see Archiva 1.0 final out before ApacheCon US
> (starting on November 10th, 2007)
>
> This code is a bug fix for current open and assigned jiras, it is not
> a rearchitecting, or work against future issues, or even an attempt at
> code perfection.
>
> Archiva is a web application, and as such, will have a different way
> of handling changes to the codebase, it is not subject to the the same
> stringent commit and change controls in place for maven, or java libs,
> or components that are either released, shared, or in common use across
> many users or other applications. This is the best time to make these
> kind of changes, before the release, before the web services.
>
> I acknowledge that I could have branched this, which given the flak i'm
> getting for this commit, I probably should have. But I stand by this
> change as a means to get the product stable in a faster fashion.
>
> I apologize for scaring the crap out of the other devs.
> I apologize for for not making a branch first.
> I apologize for not splitting up the commit.
> I apologize for stomping across other peoples active work.
>
> In the future I promise to be more clear about the scope of changes
> that need to be done.
>
> I promise to better manage my commit sizes and scope in the future.
>
> Hopefully, this has addressed any concerns about this monster commit.
>
> If not, please, by all means, SAY SOMETHING. ;-)
>
> In the end, I'm sure every developer and commitor for Archiva will
> appreciate this change for what it is.
>