On Thu, Jun 5, 2014 at 3:51 PM, Jeremie BOUSQUET <[email protected] > wrote:
> Hi, > > > 2014-06-05 15:10 GMT+02:00 Denis Gervalle <[email protected]>: > > > 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. > > > > > > > > > > > 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 ? > > > > > If I understood, you depend on B, and by transitivity, you also depend on A > (indirectly). > Then, from your module you use some classes of A (brought to you > indirectly, so "magically", it compiles). > So you never explicitly declare the dep on A from your module. If one day > you remove dependency on B for any reason, you will also loose A (and also > you can say it's an explicit dependency that is not "explicit", though it's > a matter of taste or point of view to consider transitive deps not to be > explicit ...). > > The opposite would be to declare explicit dep on A from your module, while > you don't need it at all - only B needs it. > You get it fully. The question is, could we define in a few word when the transitivity could be used, because the usage of A is just "because of" B ? Else, we should add a deps on A as soon as we import any class from A IMO. > > > > > > > > > > > > > > > > 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. > > > > > > > > > > > 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. > > > > 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. > > > > I'm not sure it relates to what you describe above. Factoring deps in a > common infrastructure is more a matter of using dependencyManagement at > correct level (top-most usually), but the rule to declare only deps that > are really needed per module is a very widely used best practice. > > From a contributor (not committer) point of view, it's sometimes annoying > to have to update your builds because of this kind of changes. Obviously it > would be difficult to consider those dependency graphs as APIs :) , but at > the same time when they evolve too frequently, it can be considered painful > to follow by devs in general ... > Some are not impacting, but for example regarding @Unstable, if it's not > brought anymore by xwiki-commons-component-api, it will break my build when > I upgrade xwiki versions. > This is bad, since I do not see any reason for xwiki-commons-component-api to bring xwiki-commons-stability to you. > > With Maven, it's not really a "bad" practice, to consider that some poms > are used mainly to bring to you a set of logically related dependencies by > transitivity (there's even the "import" scope, though it's not the best > sample of good practice I agree). Question is more (case by case), is it > really "bad" to bring @Unstable by default to everyone that develops an > xwiki component, even if he will never use it / don't use it anymore ? > I would be positive, especially because it is not a runtime deps, and we can simply use the "provided" scope. Thomas do not seems to agree. All I really want at the end, is that we have clear rule. Unless we can define what "because of" is, the only rule I see valid currently, would be to explicitly depends on any module we do an import from. > > > > > > > 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 > > > _______________________________________________ > 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
