Add `Async` Documentation #204

evilsoft · 2018-02-13T04:01:31Z

At the SAME time

This PR is mostly centered around adding the Async docs to address this issue.

Like many of the other docs added, I took the time to address some legacy error tests that have been providing false positives. Will need to circle back on some of the spec updates for some of the Monoids that have already been documented. TBH I should have done a separate PR to address the specs, and WILL do so in the future to keep review time down. Also corrected a spelling error in the Maybe docs which really should have ALSO been its own PR.

coveralls · 2018-02-13T04:06:17Z

Coverage remained the same at 100.0% when pulling 7981022 on async-docs-specs into 8fba9fd on master.

HenriqueLimas

These docs are amazing. I found some little typos and add some questions, maybe not related to that PR.

HenriqueLimas · 2018-02-13T11:42:53Z

src/Async/Async.spec.js

@@ -118,36 +119,37 @@ test('Async Resolved', t => {
 })

 test('Async fromPromise', t => {
-  const resProm = x => new Promise((res) => res(x))
+  const resProm = x => new Promise(res => res(x))

  t.ok(isFunction(Async.fromPromise), 'is a function')
  t.ok(isFunction(Async.fromPromise(resProm)), 'returns a function')

  const fn = bindFunc(Async.fromPromise)


Just curious, any reason to use sometimes fn, sometimes a and sometimesm for the variable to test?

Typically I use fn for a function, you will see that mostly with error testing functions.
Now a and m should probably, eventually be replaced with just m...I use m to represent a given ADT.

HenriqueLimas · 2018-02-13T11:47:17Z

src/Async/README.md

+difference, the arguments used in the function that is passed to the `Promise`
+constructor are reversed in an `Async`.
+
+There are many way to represent asynchronous operations in JavaScript, and as


Small typo: ways instead of way

HenriqueLimas · 2018-02-13T11:48:08Z

src/Async/README.md

+difference, the arguments used in the function that is passed to the `Promise`
+constructor are reversed in an `Async`.
+
+There are many way to represent asynchronous operations in JavaScript, and as


Small typo: ways instead of way

HenriqueLimas · 2018-02-13T11:55:38Z

src/Async/README.md

+Depending on your needs, an `Async` can be constructed in a variety of ways. The
+typical closely resembles how a `Promise` is constructed with one major
+difference, the arguments used in the function that is passed to the `Promise`
+constructor are reversed in an `Async`.


Maybe explaining why rej and res are reversed should be nice to have. I am curious too 😄

It may not be the right place to explain it in the docs. But I can do it here: So it is typical and easier to follow when the far right parameter of a type is the portion that a covariant functor works with (map). So because of this, the Resolved is the parameter that works with map. So to keep everything lined up and easier to follow, the arguments to the functions are in the same order.

That said I think I can throw something in like:

... reversed in an Async to match the order in which Async is parameterized.

Do you think that could provide some hint to why?

Yes, it can help IMHO, but it's your call here.

HenriqueLimas · 2018-02-13T12:12:10Z

src/Async/README.md

+Async e (a -> b) ~> Async e a -> Async e b
+```
+
+Short for apply, `ap` is used to apply an `Async` instance containing a value


Any reason to not use apply instead of ap? IMHO ap can be confused. (Probably this is not related to that PR)

ap is defined by Fantasy-Land. Say we want to have "static" functions on our constructor (we do not do this in crocks, but some libs do), apply would override the Function.prototype.apply. Even though we do not implement the ap spec as defined by Fantasy-Land, it makes sense to still keep the naming 🌽sistant

Yeah, was thinking something about the apply method, but since was an object I asked it. But yeah, now make sense. 👍

HenriqueLimas · 2018-02-13T12:18:57Z

src/Async/README.md

+
+The second signature is used when any cleanup needs to be performed after a
+given `Async` is cancelled by having the function returned from `fork` called.
+The first to arguments to the signature are the same as the more common


Some little typos here (in bold):
The first two arguments to the signature are the same as the more common
signature described above but takes an additional function that can be used
for "clean up" after cancellation.

HenriqueLimas · 2018-02-13T12:23:58Z

src/Async/README.md

+argument. This value will be wrapped in a resulting `Rejected` instance, in the
+case of empty.
+
+Like all `crocks` transformation functions, `firstToAsync` has (2) possible


lastToAsync instead of firstToAsync

HenriqueLimas · 2018-02-13T18:37:28Z

@evilsoft do you think is better to put everything in one single commit?

evilsoft · 2018-02-13T19:06:42Z

@HenriqueLimas Not really, what I should have done, to make it easier to review, is put the Specs in its own commit (well really its own PR),
But until a PR is made, I usually keep it to one. This makes rebasing and other branch management tasks easier.

Once I PR it publicly it though, I never amend. That way:

Reviewer never has to kill their local before pulling down a force push
Easier to review a commit to see what was changed after feedback was addressed.
it gives a nice breadcrumb trail on changes from PR feedback.

But I still rebase if there are merge conflicts during the review cycle.

evilsoft · 2018-02-14T03:33:23Z

Okay going to call this done. Merging.

update Async specs for better error testing

eb05dea

Add Async Docs

f62e294

evilsoft force-pushed the async-docs-specs branch from 033bbe8 to f62e294 Compare February 13, 2018 04:20

HenriqueLimas requested changes Feb 13, 2018

View reviewed changes

update with PR feedback and add some reviewer to all-contrib

00fdf1a

HenriqueLimas approved these changes Feb 13, 2018

View reviewed changes

moar feedback addressing

7981022

evilsoft merged commit 85d549a into master Feb 14, 2018

evilsoft deleted the async-docs-specs branch February 14, 2018 03:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Async` Documentation #204

Add `Async` Documentation #204

evilsoft commented Feb 13, 2018

coveralls commented Feb 13, 2018 •

edited

Loading

HenriqueLimas left a comment

HenriqueLimas Feb 13, 2018

evilsoft Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas Feb 13, 2018

evilsoft Feb 13, 2018 •

edited

Loading

evilsoft Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas Feb 13, 2018

evilsoft Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas Feb 13, 2018

HenriqueLimas commented Feb 13, 2018

evilsoft commented Feb 13, 2018 •

edited

Loading

evilsoft commented Feb 14, 2018

Add Async Documentation #204

Add Async Documentation #204

Conversation

evilsoft commented Feb 13, 2018

At the SAME time

coveralls commented Feb 13, 2018 • edited Loading

HenriqueLimas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evilsoft Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HenriqueLimas commented Feb 13, 2018

evilsoft commented Feb 13, 2018 • edited Loading

evilsoft commented Feb 14, 2018

Add `Async` Documentation #204

Add `Async` Documentation #204

coveralls commented Feb 13, 2018 •

edited

Loading

evilsoft Feb 13, 2018 •

edited

Loading

evilsoft commented Feb 13, 2018 •

edited

Loading