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 ?


>
> >
> > 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.

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

Reply via email to