On 16.12.2015 11:15, Martin Kosek wrote: > 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.
I though that we want to make it easy to contribute, so why are you talking about core developers? Shouldn't we make it easy to self-review own patches for everyone? Including random people who want to submit few patches and go away? (Think how we can apply usability principles to development.) Petr^2 Spacek > 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
