On 11/17/2011 05:56 AM, Vincent Massol wrote: > Hi Denis, > > Just looked at the merged code (btw I can't find how to comment on it on > github, it's a big mess), so I'm commenting here: > > I noticed 2 issues: > > * One is that even though I said in the vote mail that I didn't agree about > keeping parameters in EntityReferences you've still kept them. Again I think > we shouldn't keep them because we don't have an agreement about them ATM > (Caleb is against them). Also you've exposed them with a protected method > which means users can extend EntityReference and use them. As I said FTM I > think you should move the notion of Locale **only** in DocumentReference > since this is the only thing we've agreed about so far.
I agree that ideally locale would be set as a private final field in DocumentReference. I suggested switching it to protected since they are dangerous (at risk for being removed entirely) and that would offer some protection while we decide what to do. It is worth noting that there are a lot of dangerous protected methods including ones which violate the immutability of the reference, something else which I would ideally like to see done away with. Github seems to be displaying the merge incorrectly, you can see the original commits here: https://github.com/xwiki/xwiki-platform/commits/feature-immutable-refs Caleb > > * You have some comments in DocumentReference like this: > > * @param locale the new locale for this reference, if null, locale is > removed > > I don't understand what this means. How can it be removed since they are > immutable and it's in a constructor :) > > Also looking at setLocale I see: > > protected void setLocale(Locale locale) > { > if (locale != null) { > setParameter(LOCALE, locale); > } > } > > I don't see any removal. > > Is it a leftover/typo? > > Thanks > -Vincent > > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
