On 05/26/2015 09:25 AM, Lukas Slebodnik wrote:
On (25/05/15 17:55), Nikolai Kondrashov wrote:
> On 05/25/2015 11:44 AM, Lukas Slebodnik wrote:
>> In most cases you just need just AC_SUBST for HAVE_$sth
>> or just AC_CHECK_PROG.
>
> Yes this can make things simpler and more reliable. I remade the configuration
> to just use shell variables, as AC_SUBST is not necessary, because we don't
> use these in any configure-processed files (e.g. Makefile.am's).
>
> Please see the first attached patch to see what I mean.
>
I did few other improvements in autoconf code.
@see attached diff
Thank you, I integrated them.
> Ah, good catch, yes. Thank you, Lukas. However, leaving the
configure option
> named "--enable-intgcheck" will be confusing, so I tried to rename it to
> something more relevant to what it will be doing now. It still feels a bit
> awkward, but perhaps it's good enough.
>
> Please take a look at the second attached patch, for illustration.
>
You can also change related parts in CI script
Yeah, I did, thank you.
>> D) please find better name for
"src/tests/intg/misc.py"
>
> I think "misc.py" is perfectly fine, but since our previous discussion on
> similar matter stalled, please suggest a name and I'll name it as you say and
> we'll be done with it.
>
It's better to do not mix utility functions.
So feel free to split file to subfiles if it will be used among many files
or include helper function directly to module. You can later extract helper
function to another utility module when there will be use case.
(run_shell is not used atm)
And here are proposed names. string_utils | utils_string | utils.string,
shell_utils (python already has shutil) ...
Hmm, you asked me to change the "misc.py" name, but now you're suggesting
we
should split it. Is it really worth it for three tiny functions? What if we
just rename it to "util.py", or some other name which you find more suitable?
There will always be functions which don't fit anywhere and are not worth
creating a separate module for, and "util.py" or "misc.py" is a good
and
traditional place for these.
We can spend a week deciding where to put them and the effect on code
quality will be negligible. Can we just leave them in one module, rename it as
you see fit and have this merged sooner?
It might be better idea to do nat catch excetion (or rethrow
exception)
to see full stack trace with failure.
Hmm, I'm doing neither I think, but perhaps I'm missing or misunderstanding
something. Care to elaborate?
> AC_PATH_PROG([SLAPD], [slapd], ,
> [$PATH$PATH_SEPARATOR/usr/sbin$PATH_SEPARATOR])
> AC_SUBST([SLAPD])
AC_PATH_PROG uses AC_SUBST so you needn't it here.
Indeed, removed. Couldn't find this in documentation, though.
> -AC_DEFUN([AM_ENABLE_INTGCHECK], [
Feel free to rename macro but it shoudl not have a prefix AC_ or
AM_. It is a "namespace" for macros from autoconf or automake.
Sure, I used your version.
> AM_INTGCHECK_REQ([HAVE_PYTEST], [pytest])
> AM_INTGCHECK_REQ([HAVE_PY2MOD_LDAP], [python-ldap])
^^^^
Such might look good but if there is a longer 1st argument
you will need to reindent it.
Alright, I used your version.
BTW it causes many pep8 warnings in python code.
You mean similar practice in Python code causes PEP8 warnings? In Python code
in general, or in the Python code I'm currently submitting? Should we be
checking our code for these? If yes, how do I do that?
Nick