On Sun, May 22, 2011 at 12:09 PM, Henry Story <[email protected]> wrote: > > On 22 May 2011, at 03:35, Reto Bachmann-Gmuer wrote: > >> I commented on CLEREZZA-473 that the issue is far to broad and asked >> it to be split up, now clerezza 516 seems even broader. >> >> Both issue I think might be used as umbrella issues but are not >> suitable for actually doing commits against them. Many recent commits >> seems very hard to be reviewed. >> >> Just an example: >> >> I noted this code in a file called person_panel.java >> >> val typ: Resource = (agent/RDF.`type`).! >> return typ match { >> case FOAF.Person => personHtml(agent) >> case FOAF.Group => groupHtml(agent) >> case FOAF.Agent => agentHtml(agent) >> case _ => emptyText >> } >> >> To me this code clearly stinks, we should just call render(agent, >> "somemode") > > That's fine with me. Why not patch that? After all the point initially > of putting this up is to get feedback, and how would I have known that you > prefer that? That's what reviewing is for. But if the patch is small (max a few hours work) the issue is more likely to be detected soon which reduces the risk the same suboptimal pattern gerts repeated in various places thus reducing the risk of a frustrating veto at the end.
> I am happy to do that - a priori- though perhaps not in the next day or > two as I have to do a video of Clerezza for the presentation at the W3C > http://www.w3.org/2011/identity-ws/ Identity in the Browser Workshop > where I am presenting a paper. > > http://www.w3.org/2011/identity-ws/papers/idbrowser2011_submission_22/webid.html > > Then the week after I am concentrating on the Berlin workshop on > > http://d-cent.org/fsw2011/ > > >> and have different renderlets for the different type of >> agents. I wanted to see where this was introduced. I see the file has >> 4 commits, two of them are not associated to an issue and the other >> are associated with the very vague and broad and both still open >> issues 473 and 516. > > Nobody had left any comments or anything on CLEREZZA-516, so I narrowed > down the name to match the patch. Of course 516 is open since what is > being built is a profile viewer. This doesn't affects the problem of having files added by a process other that 1. small and well-defined issue 2. concise patch for that issue 3. closing that issue 4. reviewing [5. back to step 2] While I'd like this process to be the default I recognise that sometimes a more exploratory spike is more suitable. In this case the code must not be committed to trunk but to an issue branch. From there the code can go to trunk either as above (i.e. you can now identify the small issues leading to your goal and create the small issues and post patch for it) or as a whole, but in this case I think we should have the vote before the actual commit to trunk (basically switching to Review-The-Commit for large patches). Reto > > Henry
