On Wed, May 28, 2008 at 10:59 AM, Peter Arrenbrecht <[EMAIL PROTECTED]> wrote: > On Tue, May 27, 2008 at 7:31 AM, Germán Póo-Caamaño <[EMAIL PROTECTED]> wrote: >> Hello, >> >> This is my second report as Summer of Code student. These comments are >> very basics and the changes I made were very small. >> >> * Today I started to work. >> * I have set up an account in http://gsoc.review-board.org/ where >> I will upload my changes. It was a very nice interface to see >> the changes inline and share the reviews with annotations and >> such. >> * I figured out why the plugin wasn't loaded properly when the >> session starts. >> * The session is started by gnome-settings-daemon, which >> is started by /bin/sh. >> * Hence, the TORTOISEHG_PATH must be defined either >> ~/.profile or ~/.xprofile. >> * I updated the wiki page >> http://tortoisehg.wiki.sourceforge.net/Nautilus > > Splendid! For next time, please get a proper sourceforge account and > log in so your changes can be attributed to you. > > Also, please add the background info about gnome-settings-daemon using > /bin/sh to the line about .profile so people know why. (Yes, I could > do this myself, but it gives you a reason to go get that SF or OpenID > account and use it. ;) > >> * In my case, I have defined in my ~/.profile: >> export TORTOISEHG_PATH=/home/gpoo/code/hg/tortoisehg-dev-gpoo >> export PYTHONPATH=/home/gpoo/lib/python:$PYTHONPATH >> * While I was trying to figured out how make it works when the >> session starts I found a bug. I thought it was related, but it >> only confused me for a while: >> * The menu shows different options whether is inside or >> outside a repository. >> * The actions were working only if the menu was called >> inside a repository. >> * The actions weren't working outside a repository, even >> if the menu called was the 'About' one. >> * I made a very small patch just to test the review-board. >> You can see/comment it at >> http://gsoc.review-board.org/r/18/ (I guess you will >> need an account if you don't have any yet). >> * Also, you can check it at >> http://www.calcifer.org/hg/tortoisehg-dev/rev/ee907cf3a19a >> but I really suggest you give a review-board try. > > OK, I've tried this reviewboard thing. But I'm not sure I'm liking it, > especially for such small patches. With more contentious changes where > numerous people comment it might pay off. But the UI is dog slow in my > Firefox 2, especially and most annoyingly when typing comments. > > If we continue using this, I suggest we establish a convention that > the "Description" field is going to be the changelog entry. Your > current description in reviewboard does not match the one in your hg > repo. > > But for the moment I'm against it. I suggest we follow the hg practice > of using the `hg email` extension to send patches to the thg-devel > list. Using reviewboard erects a barrier of entry for people who might > otherwise have commented on submitted patches. > > @TK: What's your take on this?
Unfortunately I don't have a lot of free time these days (0.4 is already severely overdued). But I see you put in a decent amount of good comments, so I'd say I'll go with whatever you say, at least for now ;-) > On to the actual review: > > I tested the patch (with my suggested change - see below) and it works OK. > But: > > Both descriptions (in reviewboard and in your repo) have no proper tag > line (for example: nautilus: fix menu for non-repo dirs\n\n...). The > customary style seems to be a tag line with a leading subsystem > prefix, no first-letter capitalization, and no full stop at the end. > Please follow this style. > >> --- a/contrib/nautilus-thg.py Sun May 04 23:20:27 2008 -0500 >> +++ b/contrib/nautilus-thg.py Tue May 27 01:14:39 2008 -0400 >> @@ -168,8 +168,13 @@ class HgExtension(nautilus.MenuProvider, >> repo = self.get_repo_for_path(path) >> cwd = os.path.isdir(path) and path or os.path.dirname(path) >> >> + if repo: >> + root = repo.root >> + else: >> + root = cwd >> + > > How about `root = repo and repo.root or cwd` to continue the style of > the above line initializing cwd? I don't really expect repo.root to > ever be empty (should always be at least "/"). > >> cmdopts = [sys.executable, self.hgproc] >> - cmdopts += ['--root', repo.root] >> + cmdopts += ['--root', root] >> cmdopts += ['--cwd', cwd] >> cmdopts += ['--command', hgcmd] > > > What are your plans for the near future? > > Cheers from your friendly GSoC mentor :) > -parren > ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Tortoisehg-develop mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tortoisehg-develop
