Skip to content

Conversation

@whitchapman
Copy link
Contributor

Please let me know if I am on the right track. I am basically replacing the comments with the raw comments from rescript-lang.

@whitchapman whitchapman mentioned this pull request Jun 11, 2022
35 tasks
@whitchapman
Copy link
Contributor Author

I was attempting to stick with doing all of the Set modules but ran into a couple of issues:

  • there is not a corresponding mdx file for belt_Set.cppo.mli [and I do not know what cppo indicates]
  • when looking at belt_MutableSet.mli, noticed inconsistency between the usage of ('k,'id) and ('value, 'id) -- I figure this should be consistent throughout the modules but that would require changing code. Please advise.

@ryyppy
Copy link
Member

ryyppy commented Jun 13, 2022

Sorry for my late review.

Great work so far! The most important thing is to remove the leading res sig for each doc string, since it would just duplicate the actual function signature.

@whitchapman
Copy link
Contributor Author

whitchapman commented Jun 13, 2022

...remove the leading res sig for each doc string, since it would just duplicate the actual function signature.

I removed all duplicate signatures from all 6 files. Seems obvious in hindsight 😄 @ryyppy

@ryyppy
Copy link
Member

ryyppy commented Jun 13, 2022

looking good now. thanks!

@ryyppy ryyppy merged commit 712a52f into rescript-lang:sync-belt-doc-headers Jun 13, 2022
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.

2 participants