Update CHANGES and remove astring #18
Conversation
We were only using it for 2 functions, both of which are in stdlib. Unfortunately they're also added in 4.03 and 4.04, and we support 4.02, so the code has been spliced in until we drop support for these early versions.
src/parse_error.ml
Outdated
| for 4.02 we can remove the following 3 functions *) | ||
| let uppercase_ascii = | ||
| let open Char in | ||
| function 'a' .. 'z' as c -> unsafe_chr (code c - 32) | c -> c |
There was a problem hiding this comment.
You can also use String.capitalize and disable the warning.
There was a problem hiding this comment.
Hasn't String.capitalize gone entirely now?
There was a problem hiding this comment.
I actually checked in 5.1 before making the comment but it seems I managed to check in 4.14.1 😂. Sorry for the noise.
| @@ -7,4 +7,4 @@ | |||
| (backend bisect_ppx)) | |||
| (flags | |||
| (:standard -w -50)) | |||
| (libraries astring result camlp-streams)) | |||
| (libraries result camlp-streams)) | |||
There was a problem hiding this comment.
Why is removing astring useful while Odoc uses it too ?
There was a problem hiding this comment.
odoc-parser is used by more than just odoc. In this particular case, since once we drop support for <=4.03 the functionality we need is entirely in stdlib, it seems just good hygiene. I'm quite keen to drop old OCaml support fairly soon (and at a major version number).
There was a problem hiding this comment.
The drop of support is not planned for this release ? Then I think this can wait.
This effectively remove a dependency from OCamlformat (the only user of the parser that I know of and don't use astring) so this is still welcome. Though, I don't think accumulating "compat" code is good hygiene.
There was a problem hiding this comment.
You are right, with the planned removal of the new compat functions, I don't see a problem with adding them.
| @@ -1,21 +1,35 @@ | |||
| 2.0.0 | |||
| ----- | |||
| # 3.0.0 | |||
There was a problem hiding this comment.
This releases adds two constructors, which is a breaking change only for users that consume the entire AST. These users will break for every changes.
The Code_block constructor is changed, which breaks all known users.
These are breaking changes but require a small amount of maintenance. Given that this library exposes an AST and that every changes are breaking changes, I'd argue that we don't need to make a major release.
Also, a version number above 2.2.0 will prevent us from merging the two projects again. For which, the discussion is still open ?
There was a problem hiding this comment.
That all changes are breaking is a choice, because we could add versioned API support if we really wanted. I don't follow the argument about merging? It was never suggested we merge the opam packages again, so I don't see why they shouldn't have independent version numbers?
There was a problem hiding this comment.
We talked about merging the git repositories ? Independent version numbers are possible but a lot of work to get right. It's also not intuitive.
Why would the two projects have independent version numbers ? Both projects evolve together, I don't see the point in the added release complexity.
There was a problem hiding this comment.
I think there is an important distinction. A comment written using odoc-parser syntax requires a particular version of the parser, not a particular version of odoc.
There was a problem hiding this comment.
This doesn't contradict giving versions to the parser according to Odoc's. "this comment is written using the parser from odoc version x".
We more often add things to the parser than we remove. I think if a comment requires a specific version of the parser, it also constraints Odoc because it'll need a recent enough version.
We were only using it for 2 functions, both of which are in stdlib. Unfortunately they're also added in 4.03 and 4.04, and we support 4.02, so the code has been spliced in until we drop support for these early versions.