← Back to team overview

do-plugins team mailing list archive

Re: Proposed merge of lp:~karol-bedkowski/do-plugins/putty into lp:do-plugins/community-future

 

Vote: Needs Fixing none
Style fixes: PuttySessionItemSource formatting should match the other two, and the strings should be localizable (use Catalog.GetString ()). Drop System.IO from your Path.Combine () call, and rather than doing a series of Path.Combine calls to make the longer path, do something like new [] { "a", "b", "c"}.Aggregate (Path.Combine) but make sure to add System.Core as a reference. Rather than combining the dir name and the file name, just use the FullName property. Don't bother sorting, the relevance engine is going to mess that up for you anyway.

In your itemsource, line 72, you have m.Groups.Count == 2, you should declare that 2 as a const int at the top of the file so that someone reading your code (like me) knows why you've a two there, and also to avoid magic numbers in general.

PuttySession: line 72; this.name = session.Replace("%20", " "); Instead of replacing, that looks like putty URL encodes the file, UrlDecode it, that way you'll have all special chars covered, not just spaces. 

The description needs made into something user friendly, and why do you implement a compareTo? If that's for sort, remove it, as well as removing sort.
-- 
https://code.launchpad.net/~karol-bedkowski/do-plugins/putty/+merge/2301
You are subscribed to branch lp:do-plugins/community-future.



References