code review planning

[Rod Widdowson]

As a final update on this:

-  All actions are now complete bar one (to do with unmodifiability and
multimaps) which has an open JIRA case (287).

- I have done some wheel kicking and come up with a modified design for the
Matcher/PolicyRequirement representation which hits Scott's issues from the
review as well as my hot button of avoiding code duplication where possible.
I have written this up in the design notes page
https://wiki.shibboleth.net/confluence/x/TgC_/

Rod



> -----Original Message-----
> From: dev-bounces at shibboleth.net [mailto:dev-bounces at shibboleth.net]
> On Behalf Of Rod Widdowson
> Sent: 01 July 2013 10:59
> To: 'Shib Dev'
> Subject: RE: code review planning
> 
> Presenting further feedback from the work following on from this review:
> 
> I have made many of the changes suggested, and annotated the wiki
> appropriately.
> 
> One change is not trivial and I am still considering the options.  I made
a
> note in the JIRA case
> (https://issues.shibboleth.net/jira/browse/IDP-287#comment-15495) about
> that.
> 
> I also took an action to look at the three potential implementation of
> AttributeRule
> 1) Flat, with explicit permit and deny rules as members
> 2) Flat with a single rule member and a boolean to flag permit or deny.
> 3) As a base class with two implementations ("PermitAttributeRule" and
> "DenyAttributeRule").
> 
> The current mainline reflects (1) and over on GitHub
> (https://github.com/rodwiddowson/java-identity-provider) there is a recent
> duplicate of SVN with a couple of branches showing the two
> implementations.
> I haven't got the tests clean, but
> 
> There's not a huge difference in the implementations or the config
parsing.
> I rather think that I prefer (3), Commit
> e3ab82a539a3ed9c10d92d969cff8f612858f2fa has the parsing change for that
> and
> 5433f71f712a87a332b4d5918ed72c652bdfbb4b (much of) the implement
> changes.
> 
> Based on this, I think that I am convinced of the need (and ease involved)
> in separation the implementations of the Policy Applicability predicate
from
> the Matcher implementation.  I want to play a little bit with the
> implementation (and critically the parsing) of this before I commit to it,
> but I'll keep the list posted.
> 
> Thanks
> Rod
> 
> > -----Original Message-----
> > From: dev-bounces at shibboleth.net [mailto:dev-
> bounces at shibboleth.net]
> > On Behalf Of Rod Widdowson
> > Sent: 22 June 2013 11:29
> > To: 'Shib Dev'
> > Subject: RE: code review planning
> >
> > Thanks to all of you who say through that review yesterday.
> >
> > I was not scribing, but I took some notes and I have transferred them up
> to
> > the wiki at https://wiki.shibboleth.net/confluence/x/94O3
> >
> > Please feel free to amend or add to them.  Further discussion should
> > probably take place in this mail thread or the next meeting.  I'll
report
> on
> > my action items in this mail thread.
> >
> > Rod
> >
> > --
> > To unsubscribe from this list send an email to dev-
> > unsubscribe at shibboleth.net
> 
> --
> To unsubscribe from this list send an email to dev-
> unsubscribe at shibboleth.net