Re: svn commit: r499420 - /incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java

[Allen Gilliland <al...@sun.com>]

Dave,

these kinds of commits are going to start making me nervous now. 
granted that this is not a change that is likely to cause any real 
problems, but i don't think it's acceptable to be committing unnecessary 
changes to the code simply to work around issues with JDO/JPA.  if 
something is broken then by all means fix it, but i don't think we want 
to hack at the code to work around implementation problems with the 
JDO/JPA backend.

this is only meant to serve as an example, i am not really objecting to 
this commit, but the other example is your comment about the websiteid 
column for the weblogcategory table.  Craig had actually changed that 
column and committed it a while back and I had to revert that change 
because it's inappropriate, so I just want to say that we need to be 
careful about what is being committed.

-- Allen


snoopdave@apache.org wrote:
> Author: snoopdave
> Date: Wed Jan 24 06:36:15 2007
> New Revision: 499420
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=499420
> Log:
> Add website to equals() and hashCode(), which is redundant but allows us to work around a JPA issue.
> 
> Modified:
>     incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java
> 
> Modified: incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java
> URL: http://svn.apache.org/viewvc/incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java?view=diff&rev=499420&r1=499419&r2=499420
> ==============================================================================
> --- incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java (original)
> +++ incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java Wed Jan 24 06:36:15 2007
> @@ -438,6 +438,7 @@
>          return new EqualsBuilder()
>              .append(getRefererUrl(), o.getRefererUrl()) 
>              .append(getWeblogEntry(), o.getWeblogEntry()) 
> +            .append(getWebsite(),o.getWebsite())
>              .isEquals();
>      }
>      
> @@ -445,6 +446,7 @@
>          return new HashCodeBuilder()
>              .append(getRefererUrl())
>              .append(getWeblogEntry())
> +            .append(getWebsite())
>              .toHashCode();
>      }
>  
> 
> 

Re: svn commit: r499420 - /incubator/roller/trunk/src/org/apache/roller/pojos/RefererData.java

[Dave <sn...@gmail.com>]

On 1/24/07, Allen Gilliland <al...@sun.com> wrote:
> these kinds of commits are going to start making me nervous now.
> granted that this is not a change that is likely to cause any real
> problems, but i don't think it's acceptable to be committing unnecessary
> changes to the code simply to work around issues with JDO/JPA.  if
> something is broken then by all means fix it, but i don't think we want
> to hack at the code to work around implementation problems with the
> JDO/JPA backend.

Yes, I agree in general and understand that and am sensitive to your
need for stability at this point. I only made that particular change
because it's redundant, should be harmless and BSC doesn't use
referrers.

> this is only meant to serve as an example, i am not really objecting to
> this commit, but the other example is your comment about the websiteid
> column for the weblogcategory table.  Craig had actually changed that
> column and committed it a while back and I had to revert that change
> because it's inappropriate, so I just want to say that we need to be
> careful about what is being committed.

Yeah, that's an unacceptable change and we need to figure out the
underlying problem in the JPA code.

- Dave