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

A compatibility test against the ocaml-mustache v2.0.0 API #64

Merged
merged 1 commit into from Jan 7, 2021

Conversation

gasche
Copy link
Collaborator

@gasche gasche commented Jan 5, 2021

This test is designed to get a sense of the backward-compatibility
impact of changes to the ocaml-mustache library.

mustache_v200.mli is exactly a copy of the Mustache interface as it
existed in the version v2.0.0, and it should not change.

mustache_v200.ml is a reimplementation of this interface using the
current ocaml-mustache library. If the library changes, this
reimplementation will have to be fixed as well; the invasiveness of
this fix can be used to estimate the invasiveness of the change for
end-users.

v2.0.0 was chosen because it does not contain an explicit AST
definition or "fold" function -- whose compatibility breaks for
basically any new language feature or representation
change (for example when adding locations in the AST).

@gasche gasche force-pushed the compatibility-test-against-v2-API branch from a7b6d39 to 2628157 Compare January 5, 2021 15:30
Copy link
Owner

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

I appreciate the attention paid to backwards compatibility, but I have to wonder if it's justified here. This library has exactly 2 dependants on opam. To me this means that it would be easier to just test & fix these users and just forget about the old interface.

In any case, I don't think the changes here will slow down the development of the library. So I do think it's fine to merge.

lib_test/compat/dune Outdated Show resolved Hide resolved

# Parsed template

## parsed
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see this file being used anywhere in the test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

dune compares this output with the output generated by running user_program.exe and fails if they differ.

This test is designed to get a sense of the backward-compatibility
impact of changes to the ocaml-mustache library.

mustache_v200.mli is exactly a copy of the Mustache interface as it
existed in the version v2.0.0, and it should not change.

mustache_v200.ml is a reimplementation of this interface using the
current ocaml-mustache library. If the library changes, this
reimplementation will have to be fixed as well; the invasiveness of
this fix can be used to estimate the invasiveness of the change for
end-users.

user_program.ml is written against this interface, and we check that
it keeps working.

v2.0.0 was chosen because it does not contain an explicit AST
definition or "fold" function -- whose compatibility breaks for
basically any new language feature or representation
change (for example when adding locations in the AST).
@gasche gasche force-pushed the compatibility-test-against-v2-API branch from 2628157 to 0066e66 Compare January 6, 2021 10:49
@gasche
Copy link
Collaborator Author

gasche commented Jan 6, 2021

Regarding backwards-compatibility:

  • I expect that several users of Mustache are using the library for one-off scripts that they do not publish on opam. Taking myself as an example, I want to use ocaml-mustache for (1) a low-key website generator and (2) ugly scripts to generate emails to people. I don't think that any of these projects would end up on opam. There is more contributor activity in the project than the dependencies suggest, which is consistent with my imagination. (Note: there are now 3 dependents, with camyll released last October.)

  • The idea of this test is not to enforce backward-compatibility forever (I agree with you that it would be silly), but to get some idea of the "amount of breakage" when we change things. I feel more confident working on a codebase when I have some way to say: well this change is probably minor, but this one will affect more users.

@gasche gasche merged commit 919b315 into rgrinberg:master Jan 7, 2021
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

2 participants