Code review comment for lp:~sdague/do-plugins/sean

> NXHosts.cs:55 Do not call UpdateItems yourself, we call that for you.

Fixed

> NXHosts.cs:77; you make two calls to Path.Combine, this is ok, but we can use some C# 3.0 sexiness to > make this cleaner.
> string nxDir = new [] {Paths.UserHome, ".nx", "config"}.Aggregate (Path.Combine);
> Hotness, right?

I suppose, except it doesn't compile under monodevelop under ubuntu 8.10

> You've also got an empty catch at line 85, Print a Log.Error, something is obviously going wrong if
> it's throwing an exception, errors should never pass silently. What here was throwing an exception?
> Is there even a need for that try/catch block?

That comes from the SSH plugin in the core directory. I don't know if it is needed or not, but if it should be changed, it should be changed there as well. I'll remove it, hoping nothing goes wrong in the process.

> NXAction.cs:66; You should check if /usr/NX/bin is on the path already, it looks like you'll be
> appending it every time Perform runs.

Fixed.

> NXAction.cs:86 I think with the new API returning an empty IEnumerable is preferred to returning
> null.
> return Enumerable.Empty<IItem> ();

Fixed.

> Looks like you do that Path.Combine to get the NX config dir more than once, can you make a static > string in NXHostItem (or somewhere you think makes sense) so you're not duplicating this? One of > the most important things Dave has taught me is that any time you write the same piece of code twice you should be very concerned.

I've now moved the path into the NXHost object, that removes the duplication.