Lots of good points! Thanks for excellent communication. CeDeROM wrote: > I can see some contradictions in your argument - you want the code to > be finished, you want small changes, you want people to test it, you > don't want to integrate it, etc. I don't know where this will lead...
I'll try to clarify. This is roughly my lifecycle model for code: 1. developer gets idea either on her own or from community discussion 2. community reviews concept (still only discussion, no code yet) 3. developer writes proof-of-concept code 4. testing of proof-of-concept is done by developer and perhaps by community 5. community reviews proof-of-concept code 6. developer reworks code until no longer proof-of-concept, but finished 7. community verifies in what is supposed to be a quick review, but often at this point there is a need to go back to 6 :( 8. code is releaseable Code is produced in 3. and 6., and one dimension of development is how that code is committed. I've written previously about what makes sense to commit. Making each commit as small as possible and containing only a *single* logical change in 3. and 6. is critical to allow 5. and 7. to be as quick and easy as possible. I haven't made that model up by myself, it's simply what I have observed in some dozens of open source projects over the last few decades. It seems to work well, and it makes a lot of sense to me. It also seems to align well with other engineering domains. > I like that you are not accepting all random patches and want to keep > the organization clean, however I thouhgt GIT is for the development, > that is expanding software step by step, not just keeping the release > code. This is a good point - git is a development tool to help with development, but: that doesn't mean that we should store random development state in openocd.git master. Git makes it very easy to store development state and works-in-progress somewhere else, where work can be easily processed and reworked, until it's finished. I consider Gerrit a good place for that, but I also think that for works-in-progress which will be in progress for a bit longer time it makes sense to create a separate branch. > What is the other way to add such big changes to the project when > the change is ready? The key to making big changes is to break them up into smaller logical changes, which can be reviewed one at a time, and included one at a time. It sounds like you are already working somehow like that. > Maybe, if you don't trust my changes (which is reasonable assumption), I can't say - I haven't looked at them yet, and as you say I want to do that before discussing them much further. > most ideas are simple in the head and hard to code being restricted > with existing design... We should always improve existing design where it is neccessary to make room for new ideas, or where it simply makes sense. Restricting new work to existing flaws creates a negative spiral, causing the code to have ever more flaws. No good. Creating a positive spiral requires always doing more than just the minimum desired for the intended goal. This is why commercial software tends to be so crappy - noone cares and always "just quickly" implements whatever minimum is neccessary. Embedded is the worst, because there are fewer developers involved, who can raise hell when someone tries to pull that off. When we work on open source projects we are leaving something behind for others to benefit from. We will just be insulting them (here, you can have this, but actually we know that it's really worthless) if we do not consistently focus on that positive spiral. > > Yes, I think that's very reasonable, and well deserved. But as you > > know, many if not all OpenOCD contributors are busy, which makes it > > very difficult to process a huge set of commits - as opposed to a > > small set. > > This is one functional change - no matter is there is more smaller > patches that change file by file (as you like?), Commits which make one logical change that is as small as possible in it's scope and in the number of lines it touches are the only ones which are easy to understand. Some commits are many lines, others are very few lines, but they should always change only one thing, and the commit message should be able to describe that change exhaustively in a few lines of text. > or less patches that group the changes (as I like) - the result > will be exactly the same. Yes, the result will be the same, but the intermediate steps are quite important, because: * small logical changes are easy to understand * they make review fast * they also make debugging fast * they can even allow automated debugging (see man git-bisect scripts) * they give a meaningful zoomed-out view of development history > This is why I suggested to concentrate on the core development This is important, I think it is a blocker for the finished SWD support. Let's look at the code. > rather than patch editing. Depending on particulars, some editing may also make sense. But I agree that it's not really the typical expected case. > The change is big, there must be more than one line of the code. Of course. It's not about numbers. A logical change can be a small change (refactor some duplicated code into a function) but affect thousands of lines for hundreds of instances of that duplicate code. Such a commit is still very easy to understand and very fast to review. > Please note that I only changed one thing in > existing structure of OpenOCD and that thing was experimental anyway. For the quite significant functional change of adding SWD support to OpenOCD I rather expect exactly the opposite - it doesn't make sense to shoehorn a new concept into code which lacks good provisions for that concept. The very first thing to do is to change surroundings so that the new concept fits in naturally. > This is why I am communicating my will to change internals and the way > I like it to be done (separate thread), there is a time for discussion > and argument between different point of view. Absolutely - my point is that for SWD we should have finished such a thread before you spent too much time on writing code. > I prefer to talk over the code Yes - I'll have a look before writing more. > I really think Gerrit should be just a filter for patches and some > buffer so MINOR changes can be introduced (i.e. typos fixes, etc). > Gerrit is not a GIT, its not branch Gerrit *is* a git repository, and it does support branches. So far we haven't been using more than one, but it can, and I think that for works-in-progress it makes a lot of sense to have branches. > but you already know how I see this - you shifted GIT/MASTER/HEAD > function info Gerrit which I don't really like. I disagree - we shifted the patches-are-reviewed-on-mailinglist function into Gerrit, and by doing that we made the whole process much easier for everyone, contributors (can push directly to gerrit) and reviewers (can easily find proposed commits and see all previous review) alike. Gerrit is by far the most advanced and useful implementation of the code review process that I've seen. > If the patches are not as supposed to be, they should be simply rejected That's the meaning I assign to -2 review score. > and sent again in another form. And that's what happens when pushing a new commit with the same Change-Id. Note that the cases "this logical change is undesired regardless of implementation" and "the *way* this logical change has been implemented is undesired" are currently indistinguishable by looking only at the review score, and are usually communicated manually in comments. I think that's fine because the former case is quite rare. > I see Gerrit as patch cleaner and review tool, not the development > branch of the project. Here is our disagreement. I've seen it successfully serve both purposes in other projects. //Peter ------------------------------------------------------------------------------ Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS, MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft MVPs and experts. ON SALE this month only -- learn more at: http://p.sf.net/sfu/learnnow-d2d _______________________________________________ OpenOCD-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openocd-devel
