Skip to content

Conversation

@KR338
Copy link

@KR338 KR338 commented May 11, 2025

A convenient quick reference more than an explanation, since the functions are mostly self-explanatory.

@wojpok
Copy link

wojpok commented May 14, 2025

I like this documentation a lot, especially the conciseness of the most of the descriptions. In general, the more to the point is the documentation, the better it is. Small consmetic changes could employed. It would be nice if every comment was a proper sentence. This could be achieved by adding verbs at the beginning like Returns, Selects, etc.. When in doubt feel free to lookup examples from other languages' docuementation.

On the other hand formatting has to be improved. Signatures and descriptions should be in separate lines. I am not sure if it is agreed on, but lines shouldn't exceed 80 character in length. Type annotations don't match Fram's syntax:

  • We use Unit instead of ()
  • Pairs are annotated as Pair X Y, but this will change soon with introduction of infix type constructors
  • Type arguments are behind type constructor (for example List Int, instead of Int List)
  • ~onError parameter is implicit, documentation does not reflect that
  • There are no effect arrows. Mind the difference between ->, ->> and ->[]
  • Type variables are usually denoted as either X, Y, ... or A, B, ...
  • Some type annotations should be double-checked, for example concatMap has wrong type signature

Other then that, there is a WIP pull requests that adds documentation to List.fram file. It would be weird if code documentation and documentation online was different, thus I strongly suggest to team up and work this out together

@KR338
Copy link
Author

KR338 commented May 26, 2025

Closing as by #12 this should be a PR to dbl. Will rewrite for #12 - compliance after the other list PR is merged.

@KR338 KR338 closed this May 26, 2025
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