Skip to content

Conversation

@goldfirere
Copy link
Contributor

This was mostly straightforward. I think it would be good to review "Fix compilation problems" and "Account for new flags".

"col": 12
},
"type": "sig type t = int val x : int type s = Baz end",
"type": "sig type t = int val x : int @@ portable type s = Baz end",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unfortunate

val rev_map : ('a -> 'b) -> 'a list -> 'b list
val filter_map : ('a -> 'b option) -> 'a list -> 'b list
val concat_map : ('a -> 'b list) -> 'a list -> 'b list
val length : 'a list -> int @@ portable
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this good or bad? I would think we only want to print this with a higher verbosity?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is bad. I think this will start popping up in a lot of places and be very confusing to users

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine actually. This is just a consequence of the stdlib being portable now

@liam923
Copy link
Contributor

liam923 commented Feb 18, 2025

I've reviewed commits up to f38ea92. The changes afterwards need review. 7f95061 and e4a336b can be skipped - they are the result of importing compiler changes.

@dkalinichenko-js dkalinichenko-js self-requested a review February 18, 2025 17:15
Copy link
Contributor

@dkalinichenko-js dkalinichenko-js left a comment

Choose a reason for hiding this comment

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

The subsequent changes look good to me.

@liam923 liam923 merged commit 78c883a into main Feb 18, 2025
2 checks passed
@liam923 liam923 deleted the rae/with-kinds-roll branch February 18, 2025 18:20
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.

5 participants