On 12/16/2015 10:02 AM, Petr Spacek wrote: > On 16.12.2015 09:53, Jan Cholasta wrote: >> On 16.12.2015 09:45, Petr Spacek wrote: >>> On 11.12.2015 15:50, Jan Cholasta wrote: >>>> Hi, >>>> >>>> On 10.12.2015 18:04, Petr Spacek wrote: >>>>> On 9.12.2015 15:30, Petr Spacek wrote: >>>>>> Hello, >>>>>> >>>>>> this patch automates some of sanity checks proposed by Petr Vobornik. >>>>>> >>>>>> 'make review' should be used in root of clean Git tree which has patches >>>>>> under >>>>>> review applied. >>>>>> >>>>>> Magic in review.sh attempts to detect nearest remote branch which can be >>>>>> used >>>>>> as diff base for review. Please see review.sh for further details. >>>>> >>>>> And here is the patch! :-) >>>> >>>> Nice, but I would rather see this in ipatool >>>> (<https://github.com/freeipa/freeipa-tools>). Or is there any obvious >>>> benefit >>>> in having this in freeipa itself that I'm missing? >>> >>> For me the obvious benefit is: >>> git clone >>> git am >>> make review >>> >>> Done. >>> >>> No need to find & learn other tool, no risk of using wrong version of the >>> tool >>> to wrong version of source tree etc. >> >> AFAIK all IPA developers are supposed to use ipatool, and it already has a > > Good to know. How does a newcomer learn about it? Honestly I never used > ipatool (or not even cloned it).
ipatool targets rather upstream members with write access, so they are hardly newcomers. But still, here you go: https://www.freeipa.org/page/Contribute/Repository#Process_tools >> start-review command, so it would better fit in there. Or we could merge >> freeipa-tools into freeipa. My point is that I don't think having half of the >> stuff in ipatool and the other half in IPA itself is a good thing to do. > > I agree with this in general. > > Would it make sense to at least have review target for make which executes > ipa-tool and if it is not installed it tells you where to grab it? > > Or possibly make ipatool submodule of ipa git tree, so there is no risk of > using wrong review tool for particular checkout? Please do not overcomplicate it :-) ipatool works nicely at the moment, it is in a separate repo with other tools where every core developer can contribute and is easy to be update. -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code