[Bug 791263] Review Request: mockito - A Java mocking framework

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=791263

--- Comment #3 from Stanislav Ochotnicky <sochotni at redhat.com> 2012-02-16 12:40:39 EST ---
(In reply to comment #2)
> >  * Instead of having a patch adding the pom, add it as additional Source1
> > (direct link to maven repo or wherever) - you can refer to it later as
> > %{SOURCE1}
> 
> The problem is that the pom that they upload into the Maven repo lacks 2
> dependencies: to ant and to junit (which in this case is not only needed for
> test scope). That's why I made my own quick pom out of the Maven repository
> one. Mockito upstream doesn't seem to use Maven themselves. They use ant, and I
> suppose this would have complicated the build process even more.
> 

Funny, most people still prefer ant over maven and think the process is more
complicated with mvn :-)

Anyway, as far as pom is concerned. You should have Source1 as original pom
from maven central and then patch on top of this pom that would only introduce
changes needed in fedora and why they are needed. 

> >  * You are not installing license. Both main package and javadoc has to have
> > %doc LICENSE (and in this case also NOTICE)
> 
> Fixed.

Good, note that you can include both on the same line (i.e. %doc LICENSE
NOTICE). Nothing wrong with separate lines, just making sure you are aware of
both options

> >  *  Instead of:
> > cp -p target/mockito-core-1.9.0.jar $RPM_BUILD_ROOT%{_javadir}/%{name}.jar
> > do:
> > cp -p target/mockito-core-%{version}.jar $RPM_BUILD_ROOT%{_javadir}/%{name}.jar
> > It will simplify updates
> 
> Fixed this and a couple of other places where the version is hardcoded.

OK

> >  * Some upstreams have complicated build systems that add certain files in
> > special places and then generate something else out of them. It means that if
> > you build "your way" you can inadvertently introduce changes into final binary
> > jars. That's why we usually (not always!) prefer to build from SCM over using
> > source jars. Best way to solve this long term: ask upstream to generate
> > -project.zip files with complete contents of SCM needed to build. I wouldn't
> > block a review on this, but I would urge you to be careful. Ugly things have
> > happened when building like this :-)
> 
> Yeah true. Maybe I should look into building with upstream's ant instead. Or
> get them to provide a correct pom somewhere.

It is not a *hard* requirement, but we usually suggest using upstream's build
method with exception of gradle or similar ugly things :-) If you have a good
reason not to use upstream build method just state it in the comment somewhere
in the spec. As far as I am concerned: you are maintainer and it's up to you to
decide best way to handle your package. You just have to be aware of ups/downs
of different decisions.

> I updated the above linked files to reflect the changes I made.

Normally during review you should raise release number and add changelog just
as you would during normal package maintenance. It shows additional skills +
it's much easier to track changes through the review process. I.e. the spec url
can stay the same, but at least srpms should be always different. 

However from your comment on IRC I looked at the sources and as far as I see
they bundle asm and cglib in org.mockito.[asm|cglib] tree. This means you have
to remove those subtrees, change classes that use "shaded" classpaths to use
original asm classpaths and finally add proper dependencies in pom.xml. Might
get complicated if APIs are different (to the point where you'd have to package
different version of cglib & asm as additional rpms)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.