18F / concourse-compliance-testing

Concourse CI assets for Compliance Toolkit
https://compliance-viewer.18f.gov/
Other
17 stars 7 forks source link

allow new projects to be added #84

Closed afeld closed 8 years ago

afeld commented 8 years ago

Builds on #85.

/cc https://github.com/18F/concourse-compliance-testing/issues/75

DavidEBest commented 8 years ago

Is this ready for review?

afeld commented 8 years ago

Yep, ready! Was waiting for #85 to be merged so the diff would be cleaner.

I fought for a bit with trying to handle non-existent last-results or Team API entries for new projects, both of which would have required custom resources. From the Concourse Slack:

no one should be treating errors as a normal part of pipeline operation

Therefore, this pull request contains a couple of workarounds:

Closes #75 by avoiding the issue.

DavidEBest commented 8 years ago

LGTM. :shipit:

I have some concerns about adding configuration flags and mingling those flags with the actual target data, but I don't have a better solution. Maybe if we just write a custom resource... :trollface:

afeld commented 8 years ago

Heh. The problem is that it's tough to determine if the resource doesn't exist because it's not there yet, or if it's because there was a misspelling in the configuration or something. I say we merge this until we figure out something better.

Another option is to say "if the fields that we need are in the targets.json (right now just links), don't bother with fetching the data from the Team API." That may get harder to determine if we end up using more fields.

DavidEBest commented 8 years ago

I agree. Better to merge this and iterate.