Skip to content

Conversation

@hdgarrood
Copy link
Collaborator

  • Say what the function is/does before talking about why you might want to use it
  • Update the reference to runConj (which no longer exists)
  • Be more specific than "for free". I've grown to dislike the phrase "for free" because it can mean too many different things in different contexts, and it's usually clearer to just say what you really mean.

-- | no-op because of newtypes. For example, if you have an `Array (Conj a)` and you
-- | want an `Array (Disj a)`, you could do `Data.Array.map (runConj >>> Disj)`, but
-- | want an `Array (Disj a)`, you could do `Data.Array.map (un Conj >>> Disj)`, but
-- | this performs an unnecessary traversal. `coerce` accomplishes the same
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it help to note that the unnecessary traversal in this case is O(n) to help show the contrast more?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that sounds good to me

Copy link
Contributor

@JordanMartinez JordanMartinez left a comment

Choose a reason for hiding this comment

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

This is definitely cleaner. LGTM!

@JordanMartinez
Copy link
Contributor

Hm... CI is failing because there isn't a test folder and its running pulp test.

@JordanMartinez JordanMartinez merged commit c0af7f4 into master Oct 13, 2020
@JordanMartinez JordanMartinez deleted the update-docs-more branch October 13, 2020 11:36
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.

3 participants