On 09/26/11 - 06:03:02PM, Christopher Alfonso wrote:
On Sep 26, 2011, at 4:31 PM, Chris Lalancette clalance@redhat.com wrote:
On 09/26/11 - 01:30:47PM, Chris Alfonso wrote:
rubygem-rspec is installed by aeolus-conductor-devel, which is required before rake tasks can be run. I've added documentation to the README to direct the user to install the aeolus-conductor-devel package before running rake rpms.
README | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/README b/README index 1a596ff..e96cf2b 100644 --- a/README +++ b/README @@ -12,8 +12,8 @@ to install and configure aeolus components. To make use of this recipe, drop it into your puppet module path or alternatively build and install the aeolus-configure rpm which contains the recipe and some wrapper scripts to configure -the aeolus components.
+the aeolus components. Before executing the rake rpms task, you +will need to install the aeolus-conductor-devel package.
That seems a bit...self-referential. I would change this to just reference the package directly:
"Before executing the rake rpms task, will need to install the rspec gem"
I thought about that, but I think that will require us to update the readme each time we add a dependency to the Rakefile. From what I gathered, we expect developers to install the devel package as a prerequisite to building. Since the configure project doesn't have the aeolus-conductor-devel spec, I didn't see it as self-referential. Agree/Disagree?
Ah, I missed it was for configure and not conductor. In any case, I think I would still reference the package directly. We shouldn't be adding new dependencies to the Rakefile often; if we do, that is a sign that we are adding too many dependencies.
On 27/09/11 08:02 -0400, Chris Lalancette wrote:
On 09/26/11 - 06:03:02PM, Christopher Alfonso wrote:
On Sep 26, 2011, at 4:31 PM, Chris Lalancette clalance@redhat.com wrote:
On 09/26/11 - 01:30:47PM, Chris Alfonso wrote:
rubygem-rspec is installed by aeolus-conductor-devel, which is required before rake tasks can be run. I've added documentation to the README to direct the user to install the aeolus-conductor-devel package before running rake rpms.
README | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/README b/README index 1a596ff..e96cf2b 100644 --- a/README +++ b/README @@ -12,8 +12,8 @@ to install and configure aeolus components. To make use of this recipe, drop it into your puppet module path or alternatively build and install the aeolus-configure rpm which contains the recipe and some wrapper scripts to configure -the aeolus components.
+the aeolus components. Before executing the rake rpms task, you +will need to install the aeolus-conductor-devel package.
That seems a bit...self-referential. I would change this to just reference the package directly:
"Before executing the rake rpms task, will need to install the rspec gem"
I thought about that, but I think that will require us to update the readme each time we add a dependency to the Rakefile. From what I gathered, we expect developers to install the devel package as a prerequisite to building. Since the configure project doesn't have the aeolus-conductor-devel spec, I didn't see it as self-referential. Agree/Disagree?
Ah, I missed it was for configure and not conductor. In any case, I think I would still reference the package directly. We shouldn't be adding new dependencies to the Rakefile often; if we do, that is a sign that we are adding too many dependencies.
-- Chris Lalancette
Updated and pushed. Thanks for taking the time to review.
aeolus-devel@lists.fedorahosted.org
-
Chris Alfonso -
Chris Lalancette