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

Makes r_params backwards compatible for UVPSpec #219

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

nkern
Copy link
Member

@nkern nkern commented Aug 28, 2019

Any new parameter added to our file formats (including PSpecContainer and UVPSpec) must have a catch in their read() functions to catch for the case that the parameter is not defined.

This wasn't caught in our normal review process b/c we don't store static UVPSpec objects in our testing directly, and create them on-the-fly in our testing suite. This was done b/c our file format and its API was actively changing, but now that things have settled down I think its a good idea to make one simple static UVPSpec file in our data directory to specifically catch these backwards compatibility errors. Therefore, an example uvp object is added to the data dir and a simple read() test is added to test_uvpspec.

addresses #220

and a read test for backwards compat testing
Copy link
Collaborator

@philbull philbull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Approved, feel free to merge!

@steven-murray
Copy link
Contributor

Looks good to me!

@nkern nkern merged commit b35a830 into master Aug 29, 2019
@nkern nkern deleted the rparams_backwards_compat branch August 29, 2019 16:46
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

Successfully merging this pull request may close these issues.

None yet

3 participants