everything #3

svozza · 2017-02-23T21:00:57Z

The only purpose of this PR is for me to have a place to ask questions about the splitting out of Maybe from the main Sanctuary code base. Ignore the contents of index.js for the moment, I pretty much just ripped the Maybe code out from Sanctuary and deleted everything unrelated.

My initial question is about how to do the tests. Look at the tests for of. Should I use Sanctuary to invoke the method or should I use Maybe['fantasyland/of].of(1)? If the latter, is it OK to create an alias like this and reuse it throughout the test:

var mOf = M.Maybe['fantasy-land/of'];

Also, I get a strange output from the toString function, which is why it's commented out:

function Just(x){__cov_WlJQeVAavBNINCJxUZrEug.f['8']++;__cov_WlJQeVAavBNINCJxUZrEug.s['31']++;return new _Maybe('Just',x);}

Looks like something to do with the code coverage.

One last thing, does CircleCI have to be enabled on this repo?

davidchambers · 2017-02-23T21:09:01Z

test/of.js

@@ -0,0 +1,17 @@
+'use strict';
+
+var M = require('..');


It would be nice to be able to write var Maybe = require('sanctuary-maybe') without needing .Maybe at the end. :)

I agree, how would we do that though? How would a use create a Just or a Nothing?

Are you suggesting I add Just and Nothing to Maybe's prototype?

We could attach Just and Nothing to Maybe. The downside would be we'd then have both S.Nothing and S.Maybe.Nothing in Sanctuary. It may not be a good idea after all. Let's stick with M for now. :)

Yeah, makes sense, there'll be plenty more iterations to nail this down. :)

davidchambers · 2017-02-23T21:14:15Z

test/of.js

+  eq(mOf.length, 1);
+  // eq(mOf.toString(), 'of :: Applicative f => TypeRep f -> a -> f a');
+
+  eq(mOf(42), M.Just(42));


I think this is the only test we need for of:

eq(Z.of(Maybe, 42), Maybe.Just(42));

davidchambers · 2017-02-23T21:16:15Z

test/internal/eq.js

+var assert = require('assert');
+
+var equals = require('./equals');
+var toString = require('./toString');


Why not require sanctuary-type-classes directly and use Z.toString?

Because I just copied and pasted this bit to get the of test working for the moment. ;)

safareli · 2017-02-24T11:18:07Z

what about fantasyland/fantasy-maybes#8 ?

svozza · 2017-02-24T11:36:51Z

Yeah, we discussed it here:

sanctuary-js/sanctuary#335

davidchambers · 2017-03-13T16:15:00Z

index.js

@@ -0,0 +1,463 @@
+(function(f) {


Let's add this:

// ,______ ______, ,________,,_____,,_____,,__________ ,__________, // | \/ | | || || || \ | | // |_, ,_| |_ _||_ || _||_, __ ||_, _____| // | \ / | / \ \ \/ / | / | | // ,_| || |_,,_/ /\ \_, \ / ,_| __ \ ,_| ___|_, // | || || || | | | | || | // |______||______||_____||_____| |____| |__________/ |__________|

davidchambers · 2017-03-13T16:15:52Z

index.js

+    'sanctuary-def',
+    'sanctuary-type-classes',
+    'sanctuary-type-identifiers'
+  ];


sanctuary-js/sanctuary#334

davidchambers · 2017-03-13T16:16:30Z

index.js

+
+  //  readmeUrl :: String -> String
+  function readmeUrl(id) {
+    var version = '0.12.0';  // updated programmatically


Let's change this to '0.0.0'.

svozza · 2017-03-14T21:56:38Z

Addressed the comments and added the Just tests, am I going in the right direction (ignore index.js for now)?

davidchambers · 2017-03-15T12:15:51Z

[A]m I going in the right direction[?]

I think so. It's a little hard for me to know what you changed. It's probably a good idea to treat your branch as append-only for the time being. :)

It might be helpful to consult sanctuary-js/sanctuary-either#3. The two pull requests should end up being very similar in many respects.

svozza · 2017-03-15T12:21:27Z

Yeah, I won't squash commits for now. The only thing I added was test/Just.js. I want to know if they're OK before I do test/Nothing.js and test/Maybe.js.

davidchambers · 2017-03-15T12:39:18Z

The only thing I added was test/Just.js.

That file LGTM. :)

davidchambers · 2017-03-16T21:24:49Z

Are you happy for me to push commits to your branch, Stefano?

svozza · 2017-03-16T21:25:27Z

Yeah, go for it.

svozza · 2017-03-18T17:29:41Z

Final tests are in now.

svozza · 2017-03-20T10:43:48Z

Looking good!

svozza · 2017-04-13T09:33:34Z

What else do we need to do here?

davidchambers · 2017-04-22T11:09:34Z

The next step, Stefano, is to merge sanctuary-js/sanctuary-identity#3. We can then release [email protected] and update this pull request to depend on it.

edahlseng · 2018-01-20T02:54:59Z

What's the status on this pull request?

davidchambers · 2018-02-04T16:29:50Z

What's the status on this pull request?

I've been waiting to find time to wrap up three pull requests on which this pull request depends:

I've found some time today. I hope to release sanctuary-maybe by the end of February. 🤞

davidchambers · 2018-05-27T16:23:51Z

I'm about to squash the commits. The original commits can be found at 7c6471b.

Co-authored-by: Stefano Vozza <[email protected]>

svozza force-pushed the svozza/everything branch from 8a47feb to 7c9ca16 Compare February 23, 2017 21:04

davidchambers reviewed Feb 23, 2017

View reviewed changes

davidchambers reviewed Mar 13, 2017

View reviewed changes

index.js Outdated

'sanctuary-def',

'sanctuary-type-classes',

'sanctuary-type-identifiers'

];

Copy link

Member

davidchambers Mar 13, 2017

Choose a reason for hiding this comment

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

sanctuary-js/sanctuary#334

davidchambers reviewed Mar 13, 2017

View reviewed changes

svozza force-pushed the svozza/everything branch from 7c9ca16 to 7a1748c Compare March 14, 2017 21:55

svozza force-pushed the svozza/everything branch from 8171b40 to 5f4b5f3 Compare March 18, 2017 18:02

davidchambers changed the title ~~WIP - Split Maybe Out From Sanctuary~~ everything Mar 19, 2017

davidchambers mentioned this pull request Apr 30, 2017

[email protected] sanctuary-js/sanctuary#388

Merged

davidchambers mentioned this pull request Feb 13, 2018

❤️ ❤️ ❤️ sanctuary-js/sanctuary#487

Open

davidchambers mentioned this pull request Apr 16, 2018

Rename Option to Maybe fantasyland/fantasy-maybes#9

Closed

davidchambers mentioned this pull request May 10, 2018

everything sanctuary-js/sanctuary-identity#3

Merged

1 task

everything

64cc690

Co-authored-by: Stefano Vozza <[email protected]>

davidchambers force-pushed the svozza/everything branch from 7c6471b to 64cc690 Compare May 27, 2018 16:31

davidchambers merged commit c0d626c into master May 27, 2018

davidchambers deleted the svozza/everything branch May 27, 2018 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

everything #3

everything #3

svozza commented Feb 23, 2017 •

edited

Loading

davidchambers Feb 23, 2017

svozza Feb 23, 2017

svozza Feb 23, 2017

davidchambers Feb 23, 2017

svozza Feb 23, 2017

davidchambers Feb 23, 2017

davidchambers Feb 23, 2017

svozza Feb 23, 2017

safareli commented Feb 24, 2017

svozza commented Feb 24, 2017

davidchambers Mar 13, 2017

svozza Mar 15, 2017

davidchambers Mar 13, 2017

davidchambers Mar 13, 2017

svozza commented Mar 14, 2017

davidchambers commented Mar 15, 2017

svozza commented Mar 15, 2017

davidchambers commented Mar 15, 2017

davidchambers commented Mar 16, 2017

svozza commented Mar 16, 2017

svozza commented Mar 18, 2017

svozza commented Mar 20, 2017

svozza commented Apr 13, 2017

davidchambers commented Apr 22, 2017

edahlseng commented Jan 20, 2018

davidchambers commented Feb 4, 2018 •

edited

Loading

davidchambers commented May 27, 2018

everything #3

everything #3

Conversation

svozza commented Feb 23, 2017 • 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

safareli commented Feb 24, 2017

svozza commented Feb 24, 2017

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

svozza commented Mar 14, 2017

davidchambers commented Mar 15, 2017

svozza commented Mar 15, 2017

davidchambers commented Mar 15, 2017

davidchambers commented Mar 16, 2017

svozza commented Mar 16, 2017

svozza commented Mar 18, 2017

svozza commented Mar 20, 2017

svozza commented Apr 13, 2017

davidchambers commented Apr 22, 2017

edahlseng commented Jan 20, 2018

davidchambers commented Feb 4, 2018 • edited Loading

davidchambers commented May 27, 2018

svozza commented Feb 23, 2017 •

edited

Loading

davidchambers commented Feb 4, 2018 •

edited

Loading