You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@clerezza.apache.org by Reto Bachmann-Gmuer <re...@trialox.org> on 2011/05/22 03:35:40 UTC

CLEREZZA-473 and 516: splitting issues / move changes to branch

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") 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.

I think for changes in trunk we should really proceed by very small
issues and avoid having the same file be affected by multiple open
issues.

If this is to be a spike then it can be broader and more explorative
but then it should clearly not be in trunk.

What do we do with the existing situation, around the
accountcontrolpanel we had a lot of commits mots of them have not been
reviewed, and because of how issues and commits were made they seem
very hard to actually be reviewed.

I think the easiest might be to move stuff to a branch and review it
as a whole when it is to be merged into trunk.

Reto

Re: CLEREZZA-473 and 516: splitting issues / move changes to branch

Posted by Reto Bachmann-Gmuer <re...@trialox.org>.
On Sun, May 22, 2011 at 12:09 PM, Henry Story <he...@bblfish.net> 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

Re: CLEREZZA-473 and 516: splitting issues / move changes to branch

Posted by Henry Story <he...@bblfish.net>.
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? 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.

Henry

Re: CLEREZZA-473 and 516: splitting issues / move changes to branch

Posted by Tommaso Teofili <to...@gmail.com>.
+1, I totally agree with Reto
Tommaso

2011/5/22 Reto Bachmann-Gmuer <re...@trialox.org>

> 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") 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.
>
> I think for changes in trunk we should really proceed by very small
> issues and avoid having the same file be affected by multiple open
> issues.
>
> If this is to be a spike then it can be broader and more explorative
> but then it should clearly not be in trunk.
>
> What do we do with the existing situation, around the
> accountcontrolpanel we had a lot of commits mots of them have not been
> reviewed, and because of how issues and commits were made they seem
> very hard to actually be reviewed.
>
> I think the easiest might be to move stuff to a branch and review it
> as a whole when it is to be merged into trunk.
>
> Reto
>