17 Nov
2011
17 Nov
'11
12:30 p.m.
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
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs