Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RunDescription and RunStatus are identical - should they be collapsed? #79

Closed
geoffjentry opened this issue Jul 23, 2018 · 4 comments
Closed

Comments

@geoffjentry
Copy link
Contributor

Curious if anyone had thoughts on this.

For practical purposes RunStatus and RunDescription appear to be the same. Does anyone oppose collapsing them into a single entity?

@tetron @mckinsel @jaeddy ?

@mckinsel
Copy link

Seems fine to me; I think they're exactly the same?

@geoffjentry
Copy link
Contributor Author

Technically they're not as one of them declares both fields as REQUIRED and one doesn't, but I can't imagine it ever being used otherwise in practical terms. I'm just going to open a PR instead of philosophizing here and then going through it again in PR form

@jaeddy
Copy link
Member

jaeddy commented Jul 24, 2018

Good catch, @geoffjentry. Not sure if RunDescription is ever used, and there's a GetRunStatus operation, so I'd vote for keeping that object. RunStatus seems to be missing a description/title, so maybe steal that from RunDescription before collapsing.

@geoffjentry
Copy link
Contributor Author

@jaeddy RunDescription gets used as part of a RunListResponse which in turn gets returned by /runs GET. Discovered that as I was fixing up our endpoints and noticed I had two identical classes :)

You're right that a proper merge of the two seems to be the best course as both have informational bits that the other does not, despite being structurally identical. I'll open a PR later today and link it to this issue

geoffjentry added a commit that referenced this issue Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants