Menu
▾
▴
Re: [Notion-devel] Atomic commits! Review patches!
|
From: Arnout E. <no...@bz...> - 2011-01-27 21:04:28
|
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. > 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 |
