Menu
▾
▴
Re: [Notion-devel] Atomic commits! Review patches!
|
From: kevin g. <kev...@gm...> - 2011-01-27 22:37:39
|
On Thu, Jan 27, 2011 at 3:04 PM, Arnout Engelen <no...@bz...> wrote: > On Thu, Jan 27, 2011 at 07:33:47PM +0100, eb...@dr... wrote: >> I inspected your commit, > > That'd be the one at http://notion.git.sourceforge.net/git/gitweb.cgi?p=notion/notion;a=commit;h=8cabc6558530b6eaf126f0d57c92a719a7284b83 > >> it was hard to find the fix. >> >> Your commit shoud be split in two commits: >> first one - code purifying (indenting, variable renames etc.) >> second one - the actual fix >> This way it would be clear what you did. > > Agreed entirely. > >> I like ion3(plus) because of every line of code was critically >> considered by Tuomo. Now we have no Tuomo, thus we should review the >> patches. And not blindly apply them. > > Agreed. All committers should review any patches before applying them, and > discuss on the mailinglist in case of doubts. > > It's useful to also check the work done by committers, and I absolutely welcome > you looking into this. > > For now we don't have many reviewers (afaics you're one of the relatively few > people here with the skills required, and you mentioned earlier you don't have > much time for this). Because of this I think it would be unpractical to > require that work done by a committer himself is always first reviewed by a > second reviewer before being committed. It's up to the committer to take > responsibility and first consult the mailinglists before making changes that > might be controversial - hence the post that started this thread ;). Aside > from that commits can of course be reviewed after commiting. Perhaps patches should go to the mailing list regardless so that potential reviewers (I'm actually thinking of myself here) can look them over as time is available. Ideally they would get acked before being committed, but there is value in late reviews as well. Also it provides a resource where potential developers can become familiar with project norms and issues. Kevin Granade > >> This includes all user contributed patches that were not part of ion3plus >> (including my patch for proportional tabs). > > Yes - I wrongfully assumed the git repo set up by gwash contained only code > already vetted by Tuomo, no additional patches. I should have been more picky > there, sorry about that. > > > Kind regards, > > Arnout > >> On Sat, 22 Jan 2011 16:30:24 +0100 >> Arnout Engelen <no...@bz...> wrote: >> > Hi, >> > >> > As some of you might have noticed, notion git is starting to get into >> > shape and i've been fixing some bugs here and there. >> > >> > >From feedback from existing Ion users, the 'elastic/proportional' >> > >tab size >> > functionality that was merged into Notion from >> > http://github.com/gwash/ion-3plus is not appreciated by everyone. I >> > don't care for it much myself either, and the code is not very neat >> > (actually one of the things I fixed was an endless loop in this code, >> > causing notion to crash and fill up a CPU). >> > >> > We should at least make this behavior optional - would anyone be >> > against just reverting it entirely? >> > >> > >> > Kind regards, >> > >> > Arnout >> > >> > ------------------------------------------------------------------------------ >> > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! >> > Finally, a world-class log management solution at an even better >> > price-free! Download using promo code Free_Logger_4_Dev2Dev. Offer >> > expires February 28th, so secure your free ArcSight Logger TODAY! >> > http://p.sf.net/sfu/arcsight-sfd2d >> > _______________________________________________ >> > Notion-devel mailing list >> > Not...@li... >> > https://lists.sourceforge.net/lists/listinfo/notion-devel >> > >> >> >> -- >> Tomáš 'ebík' Ebenlendr >> PF 2011.07332182268 >> > > > >> ------------------------------------------------------------------------------ >> Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! >> Finally, a world-class log management solution at an even better price-free! >> Download using promo code Free_Logger_4_Dev2Dev. Offer expires >> February 28th, so secure your free ArcSight Logger TODAY! >> http://p.sf.net/sfu/arcsight-sfd2d >> _______________________________________________ >> Notion-devel mailing list >> Not...@li... >> https://lists.sourceforge.net/lists/listinfo/notion-devel > > > ------------------------------------------------------------------------------ > Special Offer-- Download ArcSight Logger for FREE (a $49 USD value)! > Finally, a world-class log management solution at an even better price-free! > Download using promo code Free_Logger_4_Dev2Dev. Offer expires > February 28th, so secure your free ArcSight Logger TODAY! > http://p.sf.net/sfu/arcsight-sfd2d > _______________________________________________ > Notion-devel mailing list > Not...@li... > https://lists.sourceforge.net/lists/listinfo/notion-devel > |
