[PATCH conductor]condutor extended with hwp functionality

Jason Guiditta jguiditt at redhat.com
Mon Jul 9 18:45:41 UTC 2012


On 09/07/12 18:14 +0530, Samridh wrote:
>This patch set modfies the routes which now maps /conductor/api/hardware_profiles to /conductor/hardware_profiles.
>This is required for cli hwp functionality. This can be removed once all the api related functionality is moved to the controllers.
>
>
Samridh,
I read through the patches in this set, and assuming tests pass,
things look mostly fine.  However, I am unable to apply the set due at
least partly to whitespace issues, though I think it also needs a
rebase (or at least tell me what commit hash of master you are
building this on top of, so I can test against same).  Please enable
the git hooks that will not allow whitespace and such in your
conductor/.git/hooks dir. I would suggest enabling at least:
pre-commit
pre-rebase
commit-msg
pre-applypatch
update

Also, add the following to your ~/.gitconfig:

[color]
     diff = auto
     status = auto
     branch = auto

This will show you whitespace in red when you 'git diff' before
applying (or 'git show' an existing patch in your tree).

Fwiw, the whitespace issue was around the status line of the delet
xml.

Remaining issues/questions:

1) Why, in the controller did you change the local variable into an
instance variable? Was something broken?  If not, please revert that
change, it adds excess noise to the patch with not functional benefit.

2) Please add at specs for the view validating that the templates
render objects as you expect.  They don't have to be exhaustive, but
there should be examples elsewhere showing how to validate this.

Otherwise, this looks to be pretty good overall, if you can
rebase/resend and address those minor issues, this is probably pretty
close to ready to go in.

-j



More information about the aeolus-devel mailing list