[Freeipa-devel] [DISCUSSION] checking *lint at configure time

[Tomas Krizek]

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 think changing the default from "yes" to "autodetect" negatively
>> affects packagers, but it makes it more accessible to upstream developers.
>>
> I know it is comfortable to rely on travis with PRs but travis does not check
> on debian.
>
> And I would like to know how does the current approach limit (current) upstream
> developers.
>
> LS

-- 
Tomas Krizek

PGP: 4A8B A48C 2AED 933B D495  C509 A1FB A5F7 EF8C 4869


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20170306/c51dd5cd/attachment.sig>