On 28/02/12 09:41 -0500, Mo Morsi wrote:
On 02/28/2012 07:04 AM, Petr Blaho wrote:
>Hi,
>
>yesterday I spent a huge amount of time to find out why there are some failing rspec
tests for conductor - spec/controllers/api/images_controller_spec.rb
>
>I found that there is a bug
https://bugzilla.redhat.com/show_bug.cgi?id=796783 with
multipatch as a solution - part for conductor and part for aeolus-image-rubygem
>
>And these tests were failing with aeolus-image-rubygem installed by yum.
>
>To make them work I had to build rubygem-aeolus-image rpm from source and install it.
>
>
>So I think it would be good if developers working on this type of patches should send
an email with message that rubygem from source is needed after they pushed their patches
to repo.
>
>These type of emails should have something like [ANNOUNCEMENT conductor] or [ANN
conductor] so development community can filter these emails and notice them.
>
>What do you think?
>
Hrm, when working / running tests of the latest HEAD, I will often
just assume that I need to be working against the latest revision of
all components / dependencies (at least the ones we control).
Running the tests from any given release obviously should work against
the dependencies in the repo at the time of that release, but the
development HEAD is generally assumed to be a little more unstable,
requiring more care to keep dependencies in sync when running code
and/or tests.
That being said, I agree that we need to be better at representing the
exact dependencies any give patch will require. All in all for tests,
new code, etc, a patch should detail exactly what it needs to run
successfully including specific versions / commits of Aeolus
components and other dependencies if possible. I believe this should
address the issue at hand w/out adding the extra overhead of
additional emails (unless you feel strongly about the benefit of
those?)
Yes, if patches from _another patchset_ are needed before applying a
given fix/set, then absolutely that should be made clear in the patch
introduction (the one you get with --compose that you should be using
when sending any set that needs directions or explanation).
However, if all patches in the _current_ set are all that are needed
to test the fix, no announcement is needed, merely the explanation
referenced above. If you are not already, you should be building the
aeolus rpms yourself in these cases, which wil satisfy the 'yum
install didnt get me what I needed for aeolus-image' issue mentioned
above. For those not aware, there is a simple script already in
conductor/util that does this for you. All you have to do is cd to
util/ and run ./build-all, then wait a minute or 2 for it to finish.
I use this all the time, and rarely run into the issue you mention
because of it.
Additionally, the rule has always been that if anyone is adding an
additional external dependency. It is up to them to package it up and
make sure it gets into Fedora,
fedorapeople.org/repos/aeolus, and
anywhere else it needs to be. The patch adding the dependency to
Aeolus should not be pushed until that is the case.
Agree here, my comments above are meant only for changes that require
new _aeolus_ rpms to be built, new ones are definitely not covered
there.
-j