cki-project / kpet Goto Github PK

Consider adding instructions on running tests to README.md.

The current workaround is using xmllint after the output is rendered. But it'd be good if we check that xml generated is consistent and also more human readable, well we are talking about xml right...

Since the available kernel trees and architectures available for them should be stored in the db, the --db option should be global for all entities (i.e. tree, arch, and run), and not just run.

Please consider making --db option global.

Kpet substitutes quoted architecture name into repo URL inside yum configuration inside ks_append element. It needs to be unquoted there, otherwise URL would be incorrect.

Since Beaker job XML templates can contain internal Red Hat URLs and other protected information, they need to be hosted outside the code repo and internally. A good idea might be to have them in a separate repo, pointed at by the "--db" option.

The tool should produce the global help message if a global option or an entity is not recognized, after printing the error. Same for entity options/arguments, but the output should be entity-specific. We can skip it for the start too.

Following the discussion of the PR #11

Programs which follow PROGRAM OBJECT ACTION CLI pattern often support PROGRAM HELP and sometimes PROGRAM OBJECT HELP. E.g. git help, ip help, ip route help, ipa help, and so on. Those are often in addition to PROGRAM -h/--help. It's just what people often expect from this kind of CLI, which is distinct from the one used by classic *nix tools such as ls and find. That's what I'm referring to.

I would be OK with diverging from that for the sake of simplicity. However, I have a feeling we will need that solved eventually. If we can think of plausible other "entities" which won't need the --db option, then I'd rather leave it "entity-specific" and not make it global at all, then.

One example could be a command listing possible values for one of the kpet run generate parameters, which are hard-coded, and are not part of the database. I.e. it needs to be kpet run generate --thingy FOO and there would need to be kpet thingy list producing FOO, BAR, BAZ, but FOO, BAR, and BAZ are defined in the source code and don't need the db to be output. So kpet thingy wouldn't need the --db option. OTOH, we can require it to be supplied anyway in the case we might transition to storing the list in the database after all.

Another consideration is how to structure the (eventual) configuration file, in case the --db option is left entity-specific. Should it then be specified separately for each entity type in the configuration file too? Or should it be global there? Coming from this angle, I would say it should be global both in the configuration file and on the command line, but perhaps each entity should verify that it's present by itself. I.e. tell the parser the --db is optional, but complain if it's missing in each entity interface which requires it.

We already have information about available kernel trees in the form of the filenames of available templates. Consider implementing kpet tree list command on top of that. Alternatively we can wait until a more structured database is implemented.

The kpet tree and kpet arch should have a default action of list. I.e. kpet tree and kpet tree list should be equivalent. If that's too difficult to do, we can skip it for the start and always require the command.

Make sure kpet is supplied with actual patches, and not e.g. with HTML pages on Patchwork, which only looks like a patch somewhat, so that we know we extracted the right data from there. Perhaps implement with the help of a patch-parsing library.

It seems to be easier to just go to the database directory and invoke kpet from there, than to add it to every command line. That could save typing, battling auto-completion, and some command line space.

It would be good to also somehow detect if the specified (explicitly or by default) directory looks like the database directory (e.g. check if it has the templates subdir), and abort early on with a clear error message like "Specified database directory /foo/bar/baz doesn't look like one".

One way to do that is to e.g. have a db module with functions for accessing the database, and also a function like db_is_valid(db_path), which could then be used when verifying the command-line arguments.