[SSSD] [PATCH] CI: Improve usability of ci script

Nikolai Kondrashov Nikolai.Kondrashov at redhat.com
Mon Sep 15 18:04:20 UTC 2014


On 09/12/2014 06:18 PM, Lukas Slebodnik wrote:
> On (12/09/14 16:43), Nikolai Kondrashov wrote:
>> On 09/12/2014 10:22 AM, Lukas Slebodnik wrote:
>>> On (11/09/14 23:09), Nikolai Kondrashov wrote:
>>>> On 09/11/2014 09:16 PM, Lukas Slebodnik wrote:
>>>> Um, "shared_variables.sh" module name is too long, too generic and doesn't
>>>> really make much sense having, akin to having "code.sh".
>>>>
>>> You proposed name of module "src.sh". This name does not say anything.
>>> We are no more in 80's in DOS world. There is no more limitation for name 8+3.
>>> I prefer to have long self descriptive name.
>>
>> I wouldn't mind much having it named "source.sh", although following the
>> convention having all module symbols prefixed with module name, that would
>> make all symbols longer.
>>
> 1) "source.sh" does not say anything. Source of what?

Well, it's on par with other module names. How much information could there be
in a single word? It's not much different from "deps.sh", or "configure.sh".
However, considering the context, there's not much it could be source of.

>     "shared_variables.sh" exactly says what is in this module.

Strictly speaking, it says *accurately* what is in this module, but not
exactly (not precisely). That is, the name you propose matches the contents of
the module, but it also matches contents of a lot of other modules, which
makes it less useful.

By looking at a variable name from this module being used elsewhere, can you
say it comes from there? When adding a new shared variable what is the
criteria that would make you put it in that module, and not in some other
module?

I may, perhaps, compare it to using a shotgun to pick out a terrorist from a
crowd. You will hit him, sure, but innocent bystanders might suffer. I.e. you
will be accurate (you will hit him), but not exact (you will hit others as
well). It may be better to use a rifle to be both accurate and exact.

As another example, naming the module "shell_code.sh" would also describe it
accurately. E.g. you can't argue it's not shell code in there. But that would
hardly be useful.

> 2) What kind of convention do you talk about? You can have some conventions
> in your mind, but I don't know them. I could just guess them.

The coding conventions. The way the code in question is usually done. They are
usually discovered by looking through the code and noting patterns. It is
useful to have them documented, but that might be an overkill for a small
program. Still, it is best to keep coding conventions uniform.

In this case, all the modules (those sourced .sh files) have their symbols
start with the module name. This allows quickly identifying which module a
particular symbol comes from and seeing related symbols. This makes it easier
to ensure that symbols don't clash between modules.

>> However, now I think it wouldn't be worth much making a module just for it.
>> It would be an overkill.
>>
> It is late, you proposed it and now you change your mind.

Well, I think I was wrong, I'm sorry, and I'd like to fix my mistake.

> The idea with shared module is good, but your proposal for name isn't good.
> they are really confusing or aren't self describing.

The name "src.sh" (or "source.sh") is not very exact, sure. But I'd say it is
more exact than "shared_variables.sh".

I don't know set theory well enough, but I have a feeling that
code/variables/functions dealing with "source" (whatever that is) is a smaller
set than the set of all possible shared variables.

Still, I'd say that creating a module just for one variable (as WORK_*
wouldn't be at home there), would be an overkill. That's why I proposed moving
it to misc.sh - the place for miscellaneous smallish things.

>>>> There is no need to put WORK_DIR in a module, as only contrib/ci/run uses it,
>>>> but if you really want to have it in a module, together with SRC_DIR, please
>>>> put both of them into "misc.sh", which is just the place for mixed things
>>>> which don't have a module of their own (yet).
>>>>
>>> It was your request in mail [1] to move this variable into shared module.
>>> There isn't reason to put them into module "misc.sh". The name of module
>>> "misc.sh" evokes name "garbage" or "I don't know find proper name for module".
>>
>> Yes, it is exactly that. Such modules are inevitable. It is not worth trying
>> to categorize a handful of symbols, if each of them would need a separate
>> module with a longish name.
>>
> "Code is read more than it is written". Long name are more descriptive.
> and there isn't any problem with creating more modules with long name.
> We don't have limits for amount of modules.

Yes, we don't have practical limits for amount of modules or length of names.
However, we have limits to our attention and concentration.

If there are many small modules, it is harder to relate parts of the code to
each other as you need to switch your attention between them often, constantly
losing and acquiring module context, however small. This also requires
additional effort to pinpoint the exact module for that one tiny symbol.

If there are a few big modules, they become too noisy with weakly related
stuff, blowing up module context, making it harder to acquire, even though
you'll need to do it rarely.

Some say that there is a sweet spot in module sizes. Eric Raymond for example:
http://www.catb.org/esr/writings/taoup/html/ch04s01.html

If symbols are too short, it might be hard to remember their purpose, them
having too little information. If they're too long, it might be hard to tell
them from each other as you would need to read longer words/phrases to
identify them completely.

The symbol length should be proportional to the namespace size. And the
namespace shouldn't big, if possible. Thus symbols shouldn't be long in
general.

By this:

     It is not worth trying to categorize a handful of symbols, if each of them
     would need a separate module with a longish name.

I meant that we shouldn't apply extra effort to creating too many small
modules as that would require more small context switches, increasing effort,
and as that would increase module namespace, requiring longer names with
aforementioned drawbacks, which compounded with the convention of having
module names in front of their symbols leads to even longer symbols.

>> The purpose of such (misc.sh or util.sh) modules is to keep symbols which
>> can't be easily attributed to an existing module and are not worth creating a
>> dedicated module for. Once such a "garbage" module accumulates symbols worth
>> having in a separate module, such module should be created and these symbols
>> should be moved there.
>>
> misc is abbreviation of miscellaneous
>
> The free dictionary says:
> miscellaneous
> adj
> 1. composed of or containing a variety of things; mixed; varied
> 2. having varied capabilities, sides, etc
>
> In other words: ANYTHING.

Sure.

> "util.sh" evokes helpful functions. This name is small and good, but it is
> just an exception. In most cases, sort names aren't good. You as an author can
> know what is in module "misc.sh", but anyone else have to look into this module
> and read the content and find out what is there.
>
> Imagine:
> If anyone want to find some utility function it should look into "util.sh".
> If anyone want to add new dependency it will look into "deps.sh" (btw better
> name would be dependencies.sh). but "misc.sh" does not say anything.
> Actually it does: "I don't know (I am lazy to) find proper name for module"

Now, honestly, are functions in other modules not useful, or helpful, or would
functions in "util.sh" be more useful than functions in other modules?

Could it be that people are simply used to seeing a certain kind of functions
in a module named "util", based on their problem domains, and then associate
them with that name? Say, for system programming these could be various
memory, or file operations. But would they be the same in another domain?
Financial software, for example? I would say that the latter would have
various statistical and math functions there instead.

Thus, could it be that "util" actually often contains various uncategorized
functions that are peripheral to the main purpose of the program? Or, shall we
say, "miscellaneous" functions?

I would say, that "misc" is just a more honest name than "util", as the
functions there aren't that much more useful than functions in other modules.
Actually, quite often, there are usually quite a few odd functions there, used
only a few times, and thus, by a very limited definition, "less useful".

>>> It's better to have self descriptive name otherwise you need to look into
>>> the module itself  to find out what it actualy does.
>>
>> I agree in general, but in this case "shared_variables.sh" doesn't say much
>> more than "misc.sh", yet is actually more confusing. Why it contains only two
>> shared variables and the rest is in other modules (such as DEPS_LIST,
>> DISTRO_ID)? Or, if they all were in this single module, wouldn't it be worse?
>>
> "shared_variables" says that this module contain shared variables and nothing
> else. I can file a ticket to rename file "misc.sh" to more descriptive,
> but there is no reason to add another variables to to module with confusing
> name.

The problem is, it is not easy to come up with another name for "misc.sh",
except perhaps "util.sh", which, as I argue above, is simply more
self-deceptive.

Even if another name is found, after a while, there will arise a need to have
another "misc.sh" or "util.sh", anyway.

>>>>> Subject: [PATCH 2/2] CI: Improve usability of ci script
>>>>
>>>> Could you please make the subject more specific? It's a bit too general now.
>>>> Something like "CI: Allow running outside source root" would be better, IMHO.
>>>>
>>> The title exactly says what is a purpose of this patch.
>>
>> It describes it accurately, but not exactly enough. Of course, we cannot be
>> very exact in a commit subject with its length limitation, but we can make
>> them exact enough to help distinguish the commits, which is the purpose of
>> subjects.
>>
> One more time:
> The purpose of this patch is to improve "usability of ci script" and the title
> of this commit message says it. You should not interchange title of commit
> message with body of commit message.

Both the commit message subject and the body serve the same purpose: to help
people get the idea what the commit does and to differentiate them. In a sense
they're on the same line with the commit's diff itself. I.e. subject < message
< diff. You can use them for a different purpose, like storing ASCII art of
kittens, of course, but this way seems to be most useful to me.

>> However, I'm sure there will be more commits doing such improvements. Shall we
>> put the "Improve usability of ci script" subject on all of them? Would that make
>> it easy to distinguish them when viewing the log? I doubt that.
>>
> No, because you should implement Michal's proposal and it will need broader
> discussion about next version. It is very likely that it will be significantly
> rewritten, because current version cannot be modify easily to fulfill
> requirements.
>
> In another words, current version somehow works, but it isn't very flexible and
> it is difficult to extend it. (--tests=....)
>
> So the title of the next patch should be "CI: refactor ci script"

What about a few patches afterwards? Surely we won't be able to make it
perfectly usable in one go? Or perhaps we will stop learning and thinking
about improving it afterwards?

All I'm saying is, there is always something to improve. Actually, all we do
is improve all the time. There is no work completely separated from everything
else, or work finally complete.

I think we can do a better job naming commits (i.e. giving them subjects),
which will help us identify them later. And, "CI: Allow running outside source
root" seems to be better (more precise) than "CI: Improve usability of ci
script", IMHO. It tells more of what's being actually done.

At the other end of the spectrum is just naming each commit "Changes".
I've seen such a thing done and it was useless and sad.

>>>>> +declare LOG_DIR="$PWD"
>>>>> +declare -r WORK_DIR="$PWD"
>>>>
>>>> I would really prefer "WORK_DIR" and "LOG_DIR" being called "WORK_ROOT" and
>>>> "WORK_DIR" respectively, or something similar instead. Because LOG_DIR is
>>>> always at or below WORK_DIR and they're used to point to directories not only
>>>> containing log files, but also builds and reports.
>>>>
>>> In mail [1], you proposed either WORK_DIR or WORK_ROOT.
>>> My preference is to use WORK_DIR. It is more suitable.
>>
>> Um, I actually proposed both:
>>
>>     > +declare -r CWD_DIR="$(pwd)"
>>     > +declare LOG_DIR="$(pwd)"
>>
>>     Can you name these "WORK_ROOT" and "WORK_DIR" instead?
>>
>> Perhaps I could have made it clearer.
>>
>> I meant this:
>>
>> WORK_ROOT (CWD_DIR formerly, WORK_DIR currently) should point to the root of
>> CI output directories, i.e. the directory from where it was invoked.
>>
> Please do not mention former names. It is confusing, because the latest version
> of patch does not have them.

Sorry, apparently there can be such a thing as too much context.

> i accepted that comment and name was changed.

Thank you :)

>> WORK_DIR (LOG_DIR currently) should point to the current CI output directory,
>> i.e.  should take value of $WORK_ROOT, $WORK_ROOT/ci-build-debug and
>> $WORK_ROOT/ci-build-coverage.
>>
> WORK_DIR is alias to the directory which was script executed from. It is read
> only variable and all log files and build files should be stored relative to
> this directory. There isn't bi difference between WORK_ROOT, WORK_DIR
> WORK_DIR != CURRENT_WORK_DIR

Yes, WORK_ROOT and WORK_DIR are pretty similar, that's why I named them
similarly. But we need both of them, because we need to rebase paths to be
output.

Yes, WORK_DIR != PWD. But they're similar: PWD is for code in general, and
WORK_DIR is for CI result output.

> If you don't want to give an options to choose a name don't do it.
> You proposed it; the name was chosen. I would appreciate if you accept
> your own proposal.

I'm sorry about the confusion, but, as I tried to tell in the last mail, I
didn't give an option to choose either, I proposed a change to both:

     > +declare -r CWD_DIR="$(pwd)"
     > +declare LOG_DIR="$(pwd)"

     Can you name these "WORK_ROOT" and "WORK_DIR" instead?

"These" meaning the above two variables (as opposed to "this", meaning just
one). I should have added "respectively", perhaps, but there is only
commutative choice available when there are two variables and two names to
combine.

So, could you please consider changing them after all?

Thank you.

Nick

P.S. Lukas, we should really stop this silly feuding, both of us.
      It doesn't do us nor the project any good. I think we should just spend
      some time one on one. If we don't kill each other outright, perhaps we
      will be able to work it out. You know what? Next time I go to Czech
      Republic let's go to a pub together. The beer and food is on me. The same
      goes if you come to Finland.



More information about the sssd-devel mailing list