Code review comment for lp:~jcsackett/charmworld/trigger-test-builds

Revision history for this message
j.c.sackett (jcsackett) wrote :

So, it occurred to me as I tried to resolve the issues with "run_test_builds" that we don't actually need it. Whether or not tests run can simply be based off whether jenkins_url has been set. If it's not set, there's either an error and we shouldn't run, or we don't want to run so we didn't provide it.

I think that addresses the bulk of the comments.

As to the jenkins_url/jenkins_token problem, I'm not sure I agree with the proposed url pattern change. I think providing the http://... and the token separately allows someone working with just the deployment to easily figure out what should be changed without knowing what's going on in the code. If the token on the jenkins instance was altered, change the token, and if the url has changed, change that. Since both of those are things that I think *can* change independently, they should be set indepenently. Does that make sense to you?

« Back to merge proposal