Hello Robert, a) maybe the intentions of RequireContributorRoles and RequireDeveloperRoles are not clear:
- I did not want every developer or contributor to *have a role*, but that certain roles are taken in a project, so the code checks that at least one of the developers/contributors in a project has one of the required roles. Say you always want to have someone to be the "business engineer" in a project, so you define that this role is taken at least by one of the contributors. So I changed the error message to be: "Found no %s representing role(s) '%s'", "developer|contributor", rolesFromProperties - Why did you pull down the getRolesFromProject method? It is identical in both RequireContributorRoles and RequireDeveloperRoles except of the input list :-). b) In regards to RequirePropertyDiverges: I choose to compare against a Xpp3Dom as plugins.get( MAVEN_ENFORCER_PLUGIN ).getConfiguration() returns a Xpp3Dom which I otherwise would have had to parse again. c) Unless someone wanted to extend the new rules, I like to minimize visibility therefore I normally prefer the default (package) scope instead of protected. This allows to write unit tests easily without widening the API. Regards Mirko On Wed, Apr 18, 2012 at 23:00, Robert Scholte <[email protected]> wrote: > Mirko, > > some remarks: > IMO requireRole would mean that every developer/contributor requires at > least one role. > But the code checks if the used roles are allowed. So i'd prefer to rename > those rules. > Don't you want to log the offending contributer/developer? > > If some properties are required, I think you should verfy them first. > So if AbstractRequireRoles.requiredRoles (actually allowedRoles) is empty or > RequirePropertyDiverges.property is empty the rule should fail immediately. > > The RequirePropertyDiverges looks like a pretty flat rule, so I don't see > why need those Xpp3Dom objects, but I could be wrong here. > > -Robert > > ps. I've already made some changes to the code > ________________________________ > From: [email protected] > To: [email protected] > Date: Mon, 16 Apr 2012 21:07:07 +0000 > Subject: RE: [mojo-dev] New release of extra-enforcer-rules? > > > I'll try to have a look at it this week. > > Robert >> From: [email protected] >> Date: Sun, 15 Apr 2012 22:02:36 +0200 >> To: [email protected] >> Subject: [mojo-dev] New release of extra-enforcer-rules? >> >> Hello, >> >> I have merged back my branch into the trunk which implements three new >> rules for checking diverging properties from an ancestor project as >> well as rules for checking certain roles for developers and >> contributors are defined in a project. I have written unit tests as >> well as invoker tests and am confident they should work. >> >> Are there any plans for releasing extra-enforcer-rules-1.0-alpha-3? >> >> Regards Mirko >> >> --------------------------------------------------------------------- >> To unsubscribe from this list, please visit: >> >> http://xircles.codehaus.org/manage_email >> >> --------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email
