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/03/14 10:44:45 UTC

Re: svn commit: r1081290 - in /incubator/clerezza/trunk/parent: internal.archetype/src/main/resources/archetype-resources/ platform.accountcontrolpanel/platform.accountcontrolpanel.core/ platform.accountcontrolpanel/platform.accountcontrolpanel.core/

On Mon, Mar 14, 2011 at 10:04 AM, <bb...@apache.org> wrote:

> Modified:
> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
> URL:
> http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java?rev=1081290&r1=1081289&r2=1081290&view=diff
>
> ==============================================================================
> ---
> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
> (original)
> +++
> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
> Mon Mar 14 09:04:51 2011
> @@ -49,7 +49,7 @@ public interface Renderlet {
>                private UriInfo uriInfo;
>                private MultivaluedMap<String, Object> responseHeaders;
>                private HttpHeaders requestHeaders;
> -               private final BundleContext bundleContext;
> +               public final BundleContext bundleContext;      //public
> only to test an idea
>

Could you revert this?

-                * @return a intsance of the requested rendering services
> +                * @return a instance of the requested rendering services
>                 */
>                public <T> T getRenderingService(final Class<T> type) {
>                        return AccessController.doPrivileged(
> @@ -107,14 +107,14 @@ public interface Renderlet {
>         * engine.
>         *
>         * @param res  RDF resource to be rendered with the template.
> -        * @param context  RDF resource providing a redering context.
> +        * @param context  RDF resource providing a rendering context.
>         * @param sharedRenderingValues a map that can be used for sharing
> values
> -        * across the different renderlets involved in a rendering process
> +        * across the different Renderlets involved in a rendering process
>         * @param callbackRenderer  renderer for call backs.
>         * @param renderingSpecification  the rendering specification
>         * @param mediaType  the media type this media produces (a part of)
>         * @param mode  the mode this Renderlet was invoked with, this is
> mainly used
> -        * so that the callbackeRenderer can be claeed inheriting the mode.
> +        * so that the callbackRenderer can be claeed inheriting the mode.
>         * @param requestProperties properties of the http request, may be
> null
>         * @param os  where the output will be written to.
>         */
>

It would be good to have pure reformatings in a serparate commit


> URL:
> http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala?rev=1081290&r1=1081289&r2=1081290&view=diff
>
> ==============================================================================
> ---
> incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala
> (original)
> +++
> incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala
> Mon Mar 14 09:04:51 2011
> @@ -26,6 +26,14 @@ import _root_.scala.collection.JavaConve
>  import _root_.scala.reflect.Manifest
>
>  class RichGraphNode(node: GraphNode) extends GraphNode(node.getNode,
> node.getGraph) {
> +
> +       /* because it is tedious to wrap nodes as happens in a lot of code.
> +        *
> +        * todo: does one really need to create the graph node? Is there a
> reason this is passed ike that,
> +        * todo: or was that just a quick hack? If it is because we don't
> want to use any of the superclass implementations
> +        * todo: then it would be worth creating an interface above
> GraphNode and implementing the interface instead...
> +        */
> +        def this(node: Resource, graph: GraphNode ) = this(new
> GraphNode(node,graph))
>

Could you please create issues and not commit code with TODOs except for
issues currently being worked on (typically in an issue branch).

>     /**
>      * Operator syntax shortcut to get all objects as
> <code>RichGraphNode</code>s
>      */
> @@ -56,17 +64,14 @@ class RichGraphNode(node: GraphNode) ext
>        /**
>         * returns the lexical form of literals, the unicode-string for
> UriRef for
>         * BNodes the value returned by toString
> +        * todo: not sure this is a good symbol as it is usually a binary
> symbol, and so if it is found at the end of a line the
> +        * todo: the parsers expect the expression to go on the next line
>         */
> -       def * = {
> -               val wrappedNode = node.getNode();
> -               if (wrappedNode.isInstanceOf[Literal]) {
> -                       wrappedNode.asInstanceOf[Literal].getLexicalForm
> -               } else {
> -                       if (wrappedNode.isInstanceOf[UriRef]) {
> -
> wrappedNode.asInstanceOf[UriRef].getUnicodeString
> -                       } else {
> -                               wrappedNode.toString
> -                       }
> +       def * : String = {
> +               node.getNode() match {
> +                       case lit: Literal => lit.getLexicalForm
> +                       case uri: UriRef => uri.getUnicodeString
> +                       case wrappedNode => wrappedNode.toString
>                }
>        }
>

Especially larger commits should always be associated to an issue, for very
small code reninincing (like the if/case replacement) I think its acceptable
to have them as single commit with an descriptive comment.

Reto

Re: svn commit: r1081290 - in /incubator/clerezza/trunk/parent: internal.archetype/src/main/resources/archetype-resources/ platform.accountcontrolpanel/platform.accountcontrolpanel.core/ platform.accountcontrolpanel/platform.accountcontrolpanel.core/

Posted by Henry Story <he...@gmail.com>.
Sorry about this large commit.

I had these changes nicely seperated and was trying to follow the procedure
shown here
http://www.viget.com/extend/effectively-using-git-with-subversion/

I create a branch foaf, checked things in there, then as you had asked me to merge
I did the following:

 git checkout master  
 git svn rebase  
 git merge foaf  
 git svn dcommit

This seems to have created one massive commit, instead of just adding the oldest
as it used to do. The commit is one commit for the foaf branch.

 git svn rebase

And this then had issues with conflicts, presumably because it was applying the one large commit thinking it had to the reapply the local smaller commits on top.

Trying to see how to get out of this....

Henry

On 14 Mar 2011, at 10:44, Reto Bachmann-Gmuer wrote:

> On Mon, Mar 14, 2011 at 10:04 AM, <bb...@apache.org> wrote:
> 
>> Modified:
>> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
>> URL:
>> http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java?rev=1081290&r1=1081289&r2=1081290&view=diff
>> 
>> ==============================================================================
>> ---
>> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
>> (original)
>> +++
>> incubator/clerezza/trunk/parent/platform.typerendering/platform.typerendering.core/src/main/java/org/apache/clerezza/platform/typerendering/Renderlet.java
>> Mon Mar 14 09:04:51 2011
>> @@ -49,7 +49,7 @@ public interface Renderlet {
>>               private UriInfo uriInfo;
>>               private MultivaluedMap<String, Object> responseHeaders;
>>               private HttpHeaders requestHeaders;
>> -               private final BundleContext bundleContext;
>> +               public final BundleContext bundleContext;      //public
>> only to test an idea
>> 
> 
> Could you revert this?
> 
> -                * @return a intsance of the requested rendering services
>> +                * @return a instance of the requested rendering services
>>                */
>>               public <T> T getRenderingService(final Class<T> type) {
>>                       return AccessController.doPrivileged(
>> @@ -107,14 +107,14 @@ public interface Renderlet {
>>        * engine.
>>        *
>>        * @param res  RDF resource to be rendered with the template.
>> -        * @param context  RDF resource providing a redering context.
>> +        * @param context  RDF resource providing a rendering context.
>>        * @param sharedRenderingValues a map that can be used for sharing
>> values
>> -        * across the different renderlets involved in a rendering process
>> +        * across the different Renderlets involved in a rendering process
>>        * @param callbackRenderer  renderer for call backs.
>>        * @param renderingSpecification  the rendering specification
>>        * @param mediaType  the media type this media produces (a part of)
>>        * @param mode  the mode this Renderlet was invoked with, this is
>> mainly used
>> -        * so that the callbackeRenderer can be claeed inheriting the mode.
>> +        * so that the callbackRenderer can be claeed inheriting the mode.
>>        * @param requestProperties properties of the http request, may be
>> null
>>        * @param os  where the output will be written to.
>>        */
>> 
> 
> It would be good to have pure reformatings in a serparate commit
> 
> 
>> URL:
>> http://svn.apache.org/viewvc/incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala?rev=1081290&r1=1081289&r2=1081290&view=diff
>> 
>> ==============================================================================
>> ---
>> incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala
>> (original)
>> +++
>> incubator/clerezza/trunk/parent/rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala
>> Mon Mar 14 09:04:51 2011
>> @@ -26,6 +26,14 @@ import _root_.scala.collection.JavaConve
>> import _root_.scala.reflect.Manifest
>> 
>> class RichGraphNode(node: GraphNode) extends GraphNode(node.getNode,
>> node.getGraph) {
>> +
>> +       /* because it is tedious to wrap nodes as happens in a lot of code.
>> +        *
>> +        * todo: does one really need to create the graph node? Is there a
>> reason this is passed ike that,
>> +        * todo: or was that just a quick hack? If it is because we don't
>> want to use any of the superclass implementations
>> +        * todo: then it would be worth creating an interface above
>> GraphNode and implementing the interface instead...
>> +        */
>> +        def this(node: Resource, graph: GraphNode ) = this(new
>> GraphNode(node,graph))
>> 
> 
> Could you please create issues and not commit code with TODOs except for
> issues currently being worked on (typically in an issue branch).
> 
>>    /**
>>     * Operator syntax shortcut to get all objects as
>> <code>RichGraphNode</code>s
>>     */
>> @@ -56,17 +64,14 @@ class RichGraphNode(node: GraphNode) ext
>>       /**
>>        * returns the lexical form of literals, the unicode-string for
>> UriRef for
>>        * BNodes the value returned by toString
>> +        * todo: not sure this is a good symbol as it is usually a binary
>> symbol, and so if it is found at the end of a line the
>> +        * todo: the parsers expect the expression to go on the next line
>>        */
>> -       def * = {
>> -               val wrappedNode = node.getNode();
>> -               if (wrappedNode.isInstanceOf[Literal]) {
>> -                       wrappedNode.asInstanceOf[Literal].getLexicalForm
>> -               } else {
>> -                       if (wrappedNode.isInstanceOf[UriRef]) {
>> -
>> wrappedNode.asInstanceOf[UriRef].getUnicodeString
>> -                       } else {
>> -                               wrappedNode.toString
>> -                       }
>> +       def * : String = {
>> +               node.getNode() match {
>> +                       case lit: Literal => lit.getLexicalForm
>> +                       case uri: UriRef => uri.getUnicodeString
>> +                       case wrappedNode => wrappedNode.toString
>>               }
>>       }
>> 
> 
> Especially larger commits should always be associated to an issue, for very
> small code reninincing (like the if/case replacement) I think its acceptable
> to have them as single commit with an descriptive comment.
> 
> Reto

Social Web Architect
http://bblfish.net/