Merged
Conversation
f228bce to
170f9a9
Compare
389806c to
aafc274
Compare
goldfirere
commented
Mar 9, 2023
Closed
aafc274 to
c7ba43c
Compare
antalsz
suggested changes
Mar 9, 2023
Contributor
antalsz
left a comment
There was a problem hiding this comment.
Mostly looks good, some small notes.
2144615 to
f89b991
Compare
c7ba43c to
8dfd793
Compare
e9e25bf to
9cd7dc1
Compare
9d8dec7 to
b08f303
Compare
Contributor
Author
|
OK. Does anyone want to click Approve? |
antalsz
approved these changes
Mar 10, 2023
Contributor
antalsz
left a comment
There was a problem hiding this comment.
After talking to @goldfirere I now understand the new encoding of S with M; I want to update the comment about it but we can merge besides that!
Co-authored-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
ncik-roberts
pushed a commit
to ncik-roberts/ocaml-jst
that referenced
this pull request
Mar 13, 2023
* Add Strengthen extension for module types * Use Pmty_alias, not Pmty_ident. * Antal's suggestions * Comment improvement Co-authored-by: Antal Spector-Zabusky <antal.b.sz@gmail.com> --------- Co-authored-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
ncik-roberts
pushed a commit
to ncik-roberts/ocaml-jst
that referenced
this pull request
Mar 14, 2023
* Add Strengthen extension for module types * Use Pmty_alias, not Pmty_ident. * Antal's suggestions * Comment improvement Co-authored-by: Antal Spector-Zabusky <antal.b.sz@gmail.com> --------- Co-authored-by: Antal Spector-Zabusky <antal.b.sz@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This should set the stage for a rebase of #119. Note that there are 4 new
failwiths I added for @rleshchinskiy to fill in.(This has not been adequately tested, as I'm on my personal computer on a train. But CI should fill in that gap. I know the code compiles and passes most tests.)