
On Tue, Sep 04, 2018 at 10:33:18AM +0200, Andrea Bolognani wrote:
On Tue, 2018-09-04 at 09:55 +0200, Erik Skultety wrote:
Initially, I wanted to point out that "revision" is probably not the best name [...]
I was thinking yesterday that perhaps it would be better to use '-g GITREVISION' here instead, to leave '-r' available for future extensions, eg. when/if I get around to adding support for dependencies between projects that could be used to mean 'recursive', as in: build this one project and also all those that depend on it, the same way CentOS CI does it. Does that sound reasonable?
Yep, I've been playing with the same thought too, so '-g' sounds fine to me.
[...]
def _action_build(self, hosts, projects, revision): + if revision is None: + raise Error("Missing git revision")
see above...as I said, I still don't see a reason for requiring ^this, if there truly is a compelling reason to keep it, then I'd suggest changing the default remote name to "origin" from "upstream", because it feels more natural and intuitive to me. Even though you document this in the README and I understand the reasoning - you can name your origin whatever you want + you can clone your fork and then add an upstream remote, but I wouldn't expect that to be very common practice.
Let's just use "default", that one doesn't have any charged meaning in the git world and it's pretty accurate, too.
Fair enough.
I'll also make the whole thing optional. I still have a slightly uncomfortable feeling about it, though I can't quite put it to words... Plus unlike with libvirt going one way or another won't quite lock us into that decision forever, so I'm more willing to just try stuff ;)
Reviewed-by: Erik Skultety <eskultet@redhat.com>