Le 5 juin 2014 19:44, "Denis Gervalle" <dgl(a)softec.lu> a écrit :
On Thu, Jun 5, 2014 at 3:51 PM, Jeremie BOUSQUET <
jeremie.bousquet(a)gmail.com
wrote:
> Hi,
>
>
> 2014-06-05 15:10 GMT+02:00 Denis Gervalle <dgl(a)softec.lu>lu>:
>
> > On Thu, Jun 5, 2014 at 2:27 PM, Thomas Mortagne <
> thomas.mortagne(a)xwiki.com
> > >
>
wrote:
> >
> > > On Thu, Jun 5, 2014 at 2:14 PM, Denis Gervalle <dgl(a)softec.lu>
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.
I agree with Thomas, forced dependencies in your build are bad. The only
forced dep I ever used in my root poms is junit, but here the objective was
really to force it ;)
In this case above, it would not be a pom really part of your build, but a
pom to be used as a dependency by devs users of xwiki apis. That's why I
talked about "import" scope, it's the typical use-case for it.
But I agree that in that case it's more maintenance work, additional poms
and deps graphs, so maybe not worth it at all.
I'm not sure I'd do it if I were you, even if I 'd find it nice as
contributor ;)
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 <
> > > thomas.mortagne(a)xwiki.com>
> > > > 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
<dgl(a)softec.lu>
> > 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 <
> > > >> thomas.mortagne(a)xwiki.com
> > > >> >> wrote:
> > > >> >
> > > >> >> On Mon, Aug 15, 2011 at 9:25 PM, Vincent Massol <
> > vincent(a)massol.net>
> > > >> >> 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-dependen…
> > > >> >> >>
> > > >> >> >> 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
>> >> > devs(a)xwiki.org
>> >> >
http://lists.xwiki.org/mailman/listinfo/devs
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Thomas Mortagne
>> >> _______________________________________________
>> >> devs mailing list
>> >> devs(a)xwiki.org
>> >>
http://lists.xwiki.org/mailman/listinfo/devs
>> >>
>> >
>> >
>> >
>> > --
>> > Denis Gervalle
>> > SOFTEC sa - CEO
>> > _______________________________________________
>> > devs mailing list
>> > devs(a)xwiki.org
>> >
http://lists.xwiki.org/mailman/listinfo/devs
>>
>>
>>
>> --
>> Thomas Mortagne
>> _______________________________________________
>> devs mailing list
>> devs(a)xwiki.org
>>
http://lists.xwiki.org/mailman/listinfo/devs
>>
>
>
>
> --
> Denis Gervalle
> SOFTEC sa - CEO
> _______________________________________________
> devs mailing list
> devs(a)xwiki.org
>
http://lists.xwiki.org/mailman/listinfo/devs
--
Thomas Mortagne
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Denis Gervalle
SOFTEC sa - CEO
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs