concourse / time-resource

a resource for triggering on an interval
Apache License 2.0
44 stars 32 forks source link

Extract business logic to base package #55

Closed owenfarrell closed 4 years ago

owenfarrell commented 4 years ago

Queuing this one up for awareness.

The goal of this PR is to bring the project structure in line with other resources (e.g. github-release-resource, and more recently the registry-image-resource) which have thinner main functions responsible for deserializing/serializing content from/to the command line.

The longer term need for this change is to support more consistent testing. Unit tests that rely on the time.Now(), coupled with the additional complexity of #53 and #54, will be a challenge.

For the most part, this is a lift-and-shift of existing logic. Changes to unit tests were minimal (i.e. updating BeforeEach to use the correct types), but some unit tests for check were really testing the models and their associated validation. Those tests are now in their own suite as part of the models package.

This PR is marked as a draft until #54 is merged.

izabelacg commented 4 years ago

lgtm! 👍