-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 On 03/06/2017 02:10 PM, Lukas Slebodnik wrote: > On (06/03/17 13:49), Tomas Krizek wrote: >> On 03/06/2017 01:44 PM, Lukas Slebodnik wrote: >>> On (06/03/17 13:35), Tomas Krizek wrote: >>>> On 03/03/2017 09:22 PM, Rob Crittenden wrote: >>>>> Lukas Slebodnik wrote: >>>>>> On (03/03/17 17:07), Lukas Slebodnik wrote: >>>>>>> ehlo, >>>>>>> >>>>>>> This is a small continuation fo discussin from pull request >>>>>>> "Make pylint and jsl optional" #502[1] >>>>>>> >>>>>>> Pylint and jslint are already optional because some downstream distributions >>>>>>> does not have such packages. This is a reason why desing document[2] >>>>>>> mention configuration options for disabling them. >>>>>>> --disable-pylint --without-jslint >>>>>>> >>>>>>> Previusly (4.4) "pylint was executed" before building rpm packages. >>>>>>> This strict requirement was changed because "make lint" is executed >>>>>>> with each pull request in travis. >>>>>>> >>>>>>> It was changed in commits >>>>>>> master: >>>>>>> >>>>>>> * 5c18feaa206bbaee692fc3640b7b79c8d9d6a638 CONFIGURE: Fix detection of pylint >>>>>>> * 3f91469f327d8d9f3b27e0b67c54a4f47ad845c1 CONFIGURE: Update help message for jslint >>>>>>> * b82d285a4a75e11cc9291ecca12d2fcc26f43ed1 SPEC: Fix build in mock >>>>>>> >>>>>>> The main intention of PR#502 [1] is to make it even more optional >>>>>>> and do not fail if pylint is not installed on machine. >>>>>>> In another words, changing default value from "yes" to "autodetect". >>>>>>> I think the main reason is that it is not obvious that it is an optional >>>>>>> dependency if you run just "./configure". But that can be improved with >>>>>>> better error message. @see attachments. >>>>> I was going to go into a history of why it was required (we pushed >>>>> broken changes into master) but in retrospect that doesn't really >>>>> matter. I've been out of mainline development for some time so don't >>>>> know your current processes, but I do have a question: >>>>> >>>>> Is it expected that ./configure && make && make install will result in >>>>> the bits in all the right places? We never had that expectation before >>>>> though I know Christian has been moving in that direction. Is that an >>>>> end goal? It would be nice for developing in-tree and pushing out micro >>>>> changes onto the current, live development system. >>>> If you provide correct paths to ./configure, yes - make && make install >>>> will place all the bits in the right places. I commonly use it with >>>> DESTDIR and sshfs, so I can develop locally and deploy to a remote >>>> machine without building RPMs. >>>>> If so, does it have checks for all the runtime dependencies or will you >>>>> still have to do a bunch of work afterward the make install? >>>> It doesn't check runtime dependencies. I install the freeipa rpms once >>>> to install dependencies and then use make && make install. >>>>> I've seen discussions about making freeIPA more accessible to the >>>>> average developer, which is good, but it is just so more complex than >>>>> the typical software because it is more about integration than most big >>>>> projects. So I don't know that this is every going to really be true. >>>>> Will it help the average dev install it? Sure, but then what? Will you >>>>> support such an install? >>>>> >>>>> If you want to disable the checks for *lint that is certainly your >>>>> prerogative but I see some downsides: >>>>> >>>>> - I used to setup new dev systems all the time and this is definitely >>>>> something I'd forget to do with some frequency >>>>> - As I understand it the checks will be executed by upstream before a >>>>> change is accepted so that's good but it adds a huge delay and the >>>>> requirement of a roundtrip to fix simple mistakes (happens all the time >>>>> in OpenStack). >>>> On-PR checks can handle this. When you need to fix a linter issue, you >>>> can install the dependencies and run make lint locally. >>>>> I think my question boils down to how many people will this actually >>>>> benefit vs how much time will be lost resubmitting patches? I don't >>>>> think there is an easy answer for the first part but from my own >>>>> experience I'd expect fairly regularly for lint and pep8 errors. >>>> If someone often has this issue, the workflow can be modified to address >>>> it. For example, I've configured my repo to run to run pylint and pep8 >>>> on the modified files before the commit. >>>>> On the other hand I guess this also will have the additional advantage >>>>> that make rpms will be significantly faster if you don't enable them. >>>>> >>>>> The --disable vs --without is what bugs me most about the current >>>>> situation :-) >>>>> >>>>> So in closing I'd just say that we made those checks mandatory for a >>>>> reason. Maybe that reason is no longer applicable with all the current >>>>> automation but I'd personally prefer Lukas's suggestion of requiring >>>>> them by default but providing clear output on how to disable them if >>>>> desired. This way the average user can easily work around it and it >>>>> won't impact current developers (unless they want it to). Is that as >>>>> simple as configure; make; make install? No, but it isn't a huge leap >>>>> either. >>>>> >>>>> rob >>>>> >>>> I prefer Christian's approach that makes the project more upstream-friendly. >>>> >>> Could you explain what does "more upstream-friendly" mean? >>> It seems that we have different opinion what does it mean. >> For me, it means making the project easier to develop and install, >> without the need to check ./configure options or having to look for and >> install optional dependencies. > > I am sorry but I still did not get your point. Could you a little bit > ellaborate? In this case - build won't fail when you don't have the dependencies for linters. > And few related questions to the statement "easier to develop and install" > > A) All server part is optional. Does it mean that we should disabling server by > default or autodetect all server dependencies and do not build server if they > are missing? No. We're not discussing removing the server parts. > B) it is not just an optional dependency. I tried to explain in 1st mail > that it should be a recomended dependency. I agree it's recommended. > C) Could you explain how it will be easier to develop on debian/other > distribution if upstream does not recommend to run "make lint". It probably doesn't make it easier to develop for other distributions. But it may be easier for a new upstream contributor, when building the project doesn't require any extra dependencies.
While I think Christian's PR has some value, I don't believe it's worth to endlessly discuss the change. Our opinions differ and I don't believe the change has a major impact whether accepted or not. I propose to either +1 or -1 Christian original comment in the PR [1] and either accept or reject it based on the majority of votes. [1] - https://github.com/freeipa/freeipa/pull/502#issue-209980292 - -- Tomas Krizek PGP: 4A8B A48C 2AED 933B D495 C509 A1FB A5F7 EF8C 4869 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJYvWXFAAoJECKiqUteSUFapAYIAMG/8v+DWD0QPxQLJXVlTPpB hvjpz+ei3xwrxQIPAvGw5X0oAa3R5GRcfC8WkilBjObVz925WHFwdurrnWd2Vwub NDVKVAuVj3Ly3N5kM90y4ASs9mYVQZkqipzaPf2CD7n8ihfExYMMdGYHxF63LhVi fuHxbhYkru3e3kmvdTkj5VUI02MYMk3ogytu3bXYPLmezsUB3WeBaurowEIpe4xK AemkewPeg/HsUf6VN2vZqLCu2vKzoGta69Kz/Hnjpt2ewalywMmNa+FAHRTxNn7A 3fRlaxoluRPMBYz2JOfxnoMo5Fr69AphxSoy2spAHLJZgpIkIEwpc6Z5jWFFh64= =hfwO -----END PGP SIGNATURE----- -- 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
