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

Line 18
    if settings['run_test_builds'] != 'false':
could be expressed as a positive
    if settings['run_test_builds'] == 'true':
which also ensures tests only run under a narrow condition that we control

The test and format string appear to be missing 'token'
    http://%s/job/charm-delegator/buildWithParameters?=%s&branch=lp:foo&revno=1
I expected
    http://%s/job/charm-delegator/buildWithParameters?token=%s&branch=lp:foo&revno=1

The jenkins_url should not be configured by default. Developers will run this. Maybe someone will run it themselves. Only manage.jujucharms.com will be configured to send data to jenkins, only it will have all the information to trigger a jenkins job. The tests or develop can configure the jenkins url as needed.

It's an error to run_test_builds to true, not but not set jenkins_url. I imagine we would see ingest slow,
waiting for each call to timeout. Maybe we don't need run_test_builds, or we want a sanity check to verify charmworld has enough information. I am not sure raising an error will help though. We will only see it if we look in the log.
It might be nice to print to stdout and maybe log that the url is not configured.

jenkins_url and run_test_builds don't appear related by name. As charm test data might come from several
places, maybe we want names that describe the intent clearly. Maybe charm_test_url and run_charm_tests?

The jenkins URL is somewhat brittle. The protocol might change. I don't think we need to separate the token from the URL. Can we place a formate template in a single value?
    charm_test_url: http://1.2.3.4:8080/job/charm-delegator/buildWithParameters?token=DING-DONG&branch={branch}&revno={revno}'
and trigger_tests() might do
    charm_test_url = settings['charm_test_url']
    url = charm_test_url.format(dict(branch=charm_branch, revno=revision))
charmworld only cares about a template that defines {branch} and {revno}; Everything is Jenkin's concern.