On Thu, Jun 5, 2014 at 3:42 PM, Thomas Mortagne <[email protected]> wrote:
> On Thu, Jun 5, 2014 at 3:10 PM, Denis Gervalle <[email protected]> wrote: > > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne < > [email protected]> > > wrote: > > > >> On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <[email protected]> wrote: > >> > What happen if you also use dependency A not just because of B ? > >> > >> You put a dependency on A. > >> > > > > But you may not see that so easily when you change a few line in an > > existing module. Nothing will complains until you remove your deps to B. > > If all your are saying here is that we don't have tools to enforce > that rule then yes we don't have tool to enforce that rule but we > don't have any to apply any of the proposed rules so far. > > The only tool we have is going to force you put a dependency on each > and every import you have. Do you really want to apply that ? > If this is the best way to have a consistent usage of dependencies and to keep it up to date, why not ? > > > > > > >> > >> > How do you determine "because of B" ? > >> > >> By thinking. > >> > > > > Ok, I rephrase my question. > > Could you define what you consider a usage of dependency A because of B > and > > the opposite ? > > I'm not going to copy/paste a dictionary definition of "because of". > If you cannot define the rule, you can apply it, that's all. > It's pretty obvious to me why I'm using an API, if B is not in the > equation does my code still work ? > It is not that obvious, your code is not a monolith. > > > > > > >> > >> > > >> > And what to you think of xwiki-commons-test-component that is a deps > of > >> > xwiki-platform-core ? > >> > >> It's wrong IMO. Any forced dependency is wrong IMO. > >> > >> > Should we remove it ? > >> > >> Yes we should remove it. > >> > > > > Why do we get it ? Its removal could become an nightmare... but if we > agree > > on that, we should remove it ASAP. > > Yes if we agree on that we should remove it. You don't have such > forced dependency on commons and it does not make it a nightmare... > You missed my point, removing it now could become a nightmare, and keeping it further surely increase it. I completely agree that if we do not want any useless deps in our module, we should not add such "global" deps. > > > > > > >> > >> > What about deps for logging ? > >> > >> Depends how you use it, the logger used with @Inject is an official > >> feature of our component framework so xwiki-commons-component-api > >> should be enough. > >> > >> > And could we add xwiki-commons-stability (probably provided scope) to > a > >> > high level pom to avoid adding/removing it all the time ? (or forget > it, > >> > since it come with xwiki-commons-component-api currently) ? > >> > >> It's far from being used everywhere and there is no rule forcing to > >> use it, you set @Unstable when an API is unstable, it's not forbidden > >> to not go through @Unstable. Plus you are supposed to remove that > >> annotation after some time. > >> > > > > Ok, so not deps at any scope in any high level poms. This seems opposite > to > > what Sergiu proposed, but it would be nice to agree on a rule. > > No forced dependency if it's possible any if the submodule won't need > it. It can make sense in some very specific use cases where all > submodules will have this dependency or else they have nothing to do > here (for example dependency on rendering-api in > xwiki-platform-rendering) but it's certainly not true for something > like root commons or platform pom. > > > > > To sum up, currently I am not sure the exception rule "because of" is > clear > > enough to not create confusion. I also agree with Sergiu that we should > > list all (no warning of used deps not declared in dependency:analysis), > > this make the rule clear at least. I am not against factoring common > > infrastructure in a single place, but Thomas seems to be clearly -1. > > > > It would be nice to have more feedback from other committers ! This is > not > > a minor aspect of our best practice IMO. > > > > > >> > >> > > >> > WDYT ? > >> > > >> > > >> > > >> > On Thu, Jun 5, 2014 at 11:42 AM, Thomas Mortagne < > >> [email protected]> > >> > wrote: > >> > > >> >> For me the rule to apply is simple: when you require dependency A > >> >> because of another dependency B (B expose A in it's API, you > implement > >> >> an interface of A to be found by B, etc.) you should not explicitly > >> >> depend on A. > >> >> > >> >> On Thu, Jun 5, 2014 at 11:32 AM, Denis Gervalle <[email protected]> > wrote: > >> >> > Hi devs, > >> >> > > >> >> > I am reviving this proposal since we never came to a conclusion, > and I > >> >> have > >> >> > the feeling that our deps become more and more convoluted. > >> >> > > >> >> > To resume what I get from past history with my own vision: > >> >> > > >> >> > 1) Since our modules are getting really modular, it should never > >> silently > >> >> > depends on transitive dependency of another module (with IMO some > >> >> > exception, see 3). So any undeclared deps found by > dependency:analyse > >> >> > should be explicitly declare in the effective pom (Vincent POV as > >> well) > >> >> > 2) Apply maven principle, we should reuse and apply > >> >> > convention-over-configuration > >> >> > over configuration, so common dependency like slf4j, > >> >> xwiki-commons-stability > >> >> > ? ... should be in a high level parent pom > >> >> > 3) We may rely on some very tight transitive dependency, for > exemple, > >> it > >> >> > would be really curious that xwiki-commons-component-api stop > >> providing > >> >> > javax.inject, or that xwki-commons-test-components stop providing > >> junit, > >> >> > mockito, and al. > >> >> > > >> >> > It would be nice to add those rules in our best practice and to > always > >> >> > check our pom upon finishing changes in a module. The best would > be to > >> >> > enforce those rules, but this is not easy since static analysis is > >> >> limited > >> >> > and could create false positive. > >> >> > > >> >> > WDYT ? > >> >> > > >> >> > > >> >> > On Mon, Aug 15, 2011 at 10:31 PM, Thomas Mortagne < > >> >> [email protected] > >> >> >> wrote: > >> >> > > >> >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol < > [email protected]> > >> >> >> wrote: > >> >> >> > > >> >> >> > On Aug 12, 2011, at 4:45 PM, Sergiu Dumitriu wrote: > >> >> >> > > >> >> >> >> On 08/12/2011 07:50 AM, Vincent Massol wrote: > >> >> >> >>> Hi devs, > >> >> >> >>> > >> >> >> >>> Running mvn dependency:dependency-analyze produces interesting > >> >> results. > >> >> >> >>> > >> >> >> >>> For example: > >> >> >> >>> > >> >> >> >>> [INFO] > >> >> >> > >> >> > >> > --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > >> >> >> >>> [INFO] Building XWiki Commons - Properties 3.2-SNAPSHOT > >> >> >> >>> [INFO] > >> >> >> > >> >> > >> > --------------------------------------------------------------------------------------------------------------------------------------------------------------------------- > >> >> >> >>> … > >> >> >> >>> [INFO] --- maven-dependency-plugin:2.3:analyze (default-cli) @ > >> >> >> xwiki-commons-properties --- > >> >> >> >>> [WARNING] Used undeclared dependencies found: > >> >> >> >>> [WARNING] org.slf4j:slf4j-api:jar:1.6.1:compile > >> >> >> >>> [WARNING] javax.inject:javax.inject:jar:1:compile > >> >> >> >>> [WARNING] Unused declared dependencies found: > >> >> >> >>> [WARNING] > >> >> >> > >> org.xwiki.commons:xwiki-commons-component-api:jar:3.2-SNAPSHOT:compile > >> >> >> >>> [WARNING] > >> >> org.xwiki.commons:xwiki-commons-test:jar:3.2-SNAPSHOT:test > >> >> >> >>> [WARNING] > >> org.hibernate:hibernate-validator:jar:4.2.0.Final:test > >> >> >> >>> [WARNING] org.hamcrest:hamcrest-core:jar:1.1:test > >> >> >> >>> [WARNING] org.jmock:jmock:jar:2.5.1:test > >> >> >> >>> > >> >> >> >>> The question is (for this module but more generally for all > >> others): > >> >> >> >>> * Should we add slf4j and javax.inject reps in the pom.xml for > >> this > >> >> >> module? (for ex today slf4j and javax.inject are found in the > >> >> component-api > >> >> >> dep) > >> >> >> >>> > >> >> >> >>> I think we should, wdyt? > >> >> >> >> > >> >> >> >> +1 as well. > >> >> >> > > >> >> >> > hmm actually we need to decide about the following: > >> >> >> > > >> >> >> > * In order to simplify writing pom.xml for modules having > >> components > >> >> >> (i.e. depending on xwiki-commons-component-api) I had added the > >> >> following > >> >> >> to xwiki-commons-component-api/pom.xml: > >> >> >> > > >> >> >> > <!-- Make it easy for components that wish to log - They > don't > >> have > >> >> >> to explicitly import SLF4J --> > >> >> >> > <dependency> > >> >> >> > <groupId>org.slf4j</groupId> > >> >> >> > <artifactId>slf4j-api</artifactId> > >> >> >> > </dependency> > >> >> >> > > >> >> >> > * In the same manner we have a dependency on javax.inject in > >> >> >> xwiki-commons-component-api/pom.xml: > >> >> >> > > >> >> >> > <!-- We add this dependency here so that users of the > Component > >> API > >> >> >> just need to depend on this artifact and > >> >> >> > don't have to explicitly add a dependency on > >> >> >> javax.inject:java.inject. --> > >> >> >> > <dependency> > >> >> >> > <groupId>javax.inject</groupId> > >> >> >> > <artifactId>javax.inject</artifactId> > >> >> >> > <version>1</version> > >> >> >> > </dependency> > >> >> >> > > >> >> >> > So the question is: do we want to force each module depending on > >> >> >> xwiki-commons-component-api to have to declare an explicit dep on > >> >> >> javax.inject and org.slf4j? > >> >> >> > > >> >> >> > I'm not so sure about that… > >> >> >> > >> >> >> I'm -0 and near -1 to list this kind of dependencies, using slf4j > or > >> >> >> javax.inject are what you HAVE TO use when you write an XWiki > >> >> >> component so it's redundant from my POV. > >> >> >> > >> >> >> > > >> >> >> > WDYT? > >> >> >> > > >> >> >> > Thanks > >> >> >> > -Vincent > >> >> >> > > >> >> >> >>> Note that the "Unused declared dependencies found:" doesn't > >> always > >> >> >> generate correct results as is the case here. This is mostly > because > >> >> it's a > >> >> >> static byte code check so any dep used at runtime will be > considered > >> >> unused. > >> >> >> >>> See > >> >> >> > >> >> > >> > http://www.sonatype.com/books/mvnex-book/reference/optimizing-sect-dependency-plugin.html > >> >> >> >> > >> >> >> >> Some of these dependencies are not used directly by us, but are > >> >> needed > >> >> >> >> transitively by another library. For example, slf4j needs > logback, > >> >> which > >> >> >> >> we never use directly (although we don't really declare it in > >> every > >> >> >> >> module that does logging). Hibernate needs us to pick a cache, > a > >> >> >> >> connection pool, validator, and a bytecode manipulation > utility. > >> So > >> >> yes, > >> >> >> >> we can safely ignore most of these false negatives, but we > should > >> >> still > >> >> >> >> try to remove those that are really wrongfully declared as > >> >> dependencies. > >> >> >> >> > >> >> >> >>> Thanks > >> >> >> >>> -Vincent > >> >> >> > > >> >> >> > _______________________________________________ > >> >> >> > devs mailing list > >> >> >> > [email protected] > >> >> >> > http://lists.xwiki.org/mailman/listinfo/devs > >> >> >> > > >> >> >> > >> >> >> > >> >> >> > >> >> >> -- > >> >> >> Thomas Mortagne > >> >> >> _______________________________________________ > >> >> >> devs mailing list > >> >> >> [email protected] > >> >> >> http://lists.xwiki.org/mailman/listinfo/devs > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > -- > >> >> > Denis Gervalle > >> >> > SOFTEC sa - CEO > >> >> > _______________________________________________ > >> >> > devs mailing list > >> >> > [email protected] > >> >> > http://lists.xwiki.org/mailman/listinfo/devs > >> >> > >> >> > >> >> > >> >> -- > >> >> Thomas Mortagne > >> >> _______________________________________________ > >> >> devs mailing list > >> >> [email protected] > >> >> http://lists.xwiki.org/mailman/listinfo/devs > >> >> > >> > > >> > > >> > > >> > -- > >> > Denis Gervalle > >> > SOFTEC sa - CEO > >> > _______________________________________________ > >> > devs mailing list > >> > [email protected] > >> > http://lists.xwiki.org/mailman/listinfo/devs > >> > >> > >> > >> -- > >> Thomas Mortagne > >> _______________________________________________ > >> devs mailing list > >> [email protected] > >> http://lists.xwiki.org/mailman/listinfo/devs > >> > > > > > > > > -- > > Denis Gervalle > > SOFTEC sa - CEO > > _______________________________________________ > > devs mailing list > > [email protected] > > http://lists.xwiki.org/mailman/listinfo/devs > > > > -- > Thomas Mortagne > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > -- Denis Gervalle SOFTEC sa - CEO _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
