Hi Eoghan,
On Tue, 2010-04-20 at 14:43 +0100, Eoghan Glynn wrote:
Just pushed up a branch with subresource locator usage and singleton
VmsResource:
http://git.fedorahosted.org/git/?p=rhevm-api.git;a=commit;h=d23e20069f72f...
Looks great to me, much cleaner. Some comments/questions:
- Is there no longer a problem with multiple DummyApplication
instances being created?
- Can we move the Application class into the common api package?
- Are you using the fully qualified name for DummyTestBase.VmsResource
just for clarity?
- private VmResource getService() {
+ private DummyTestBase.VmsResource getService() {
Makes sense, just checking
None of that is a big deal, though - feel fee to push to master
Mark, since I'm not yet set up to test against powershell, can I
ask
that you cast your eye over this and give it a whirl against your local
setup?
It looks fine, I'll give it a shot when my RHEV-M install works again :)
Looking the diffs, I just realised that we've conflicting tab
settings
in Eclipse. We'll have to standardise on either 4-space or tabs.
Whatever happened to 'The first rule of code style is: you do not talk
about code style.' ? :)
Seriously, you're a java guy and I'm not - so, you should decide on the
coding style and write it up. If you give me the appropriate settings
for eclipse, I'll be happy to follow it (after converting it to emacs
settings :)
To put my oar in - in your PowerShellVmsResource.java a blank line
between the methods would make me feel better.
Also, if we can at all, it'd be nice to ban "trailing whitespace" from
the project from the start[1] - it doesn't seem to be easy to do in
eclipse, so if you think it'd be too much of a pain, nevermind
Cheers,
Mark.
[1] - in libvirt they did it post-hoc and added a commit hook to check
for it:
http://www.mail-archive.com/libvir-list@redhat.com/msg04896.html