Re: [dpdk-ci] [PATCH v2] add script to decide best tree match for patches

[Thomas Monjalon <thomas@monjalon.net>]

Hi Jeremy,
Thanks for reviewing.
Some comments below:

15/02/2019 22:28, Jeremy Plsek:
> We have a python script that we use to apply patches. With this
> current script, it's a little bit awkward to use inside our apply
> script.
> Right now it would look something like this inside of our apply script:
> (ggt = guess-git-tree)
> ggt.configure_git_pw(args)
> ggt.RULES = construct_rules()
> series = ggt.api_get('series', id)
> tree_list = list_trees_for_series(series)
> tree_list = get_ordered_matches(tree_list)
> # now attempt to apply patch to each tree until successful

I think you should call this script as a black box.

> I'd rather see this implemented in a class, so that things like the
> construct_rules() and configure_git_pw() happens in the constructor,
> then I'd like to call something like get_ordered_tree_for_series(id)
> to get the list.

The idea behind dpdk-ci.git is to allow building a CI infrastructure
by assembling a collection of scripts.
If possible, we should keep each script independent and simple to use,
in the Unix spirit.

> The constructor arguments would probably contain the
> server address and project. (Or you could make the server address also
> an environment variable like PATCHWORK_API_URL =
> os.getenv("PATCHWORK_API_URL", "http://patches.dpdk.org/api/1.0/"),
> which is what we do in our scripts.)

The URL is not going to change anytime soon.
What is the benefit of reading it from an environment variable?
I see one benefit: allow to test with a test server.
I would be in favor of providing a default URL and allow to override
with an environment variable.