Can I have it with milk? #18

blazgocompany · 2024-06-26T15:05:02Z

Yep, it's me again. I hope I'm not pestering you.

towerofnix

Hmm, this might be a contentious subject... of course you can have it with milk, and indeed any milk would probably be quite delicious. But is cinnamon milk truly the best? Is not the beauty of Cinnamon Toast Crunch in milk, that it flavors the milk, that its sugar-cinnamon goodness is absorbed into the milk? In this regard cinnamon milk is not only superfluous, but actually obscures that oh-so-essential cereal-milk action!

I think if we want to be a serious reference on why kids love the taste of Cinnamon Toast Crunch (and we surely do), then we might take care to acknowledge Cinnamon Toast Crunch in all its delightful dynamics.

We should also try to match the larger JavaScript ecosystem. getMilkCombinations... is a plural name, so one would expect it to return an array or otherwise iterable (such as a generator). Apart from the cinnamon-philosophical quandary above, either the function should be renamed (the export is OK), or the contents should return an iterable of milk combinations that go with cinnamon toast crunch.

Thank you for your contributions, of course!

blazgocompany · 2024-06-26T20:03:55Z

Great point... Ill think about it

blazgocompany · 2024-06-26T20:08:28Z

Your language in the first paragraph is truly amazing! I don't know if this is what you're looking for (see the commit) but I hope that's good!

towerofnix · 2024-06-29T14:47:02Z

index.js

+    "Soy Milk",
+    "Cinnamon Milk"
+  ];
+}


Thanks, this is almost there!! I'm a stickler for literals, so can we try a syntax that's more like...

module.exports.whichMilkCanIHaveItWith = function* getMilkCombinations() { yield "with 2% milk" yield "with whole milk" yield "with almond milk" yield "with coconut milk" yield "with soy milk" yield "with cinnamon milk" yield "with your favorite milk" yield "or even dry" }

Maybe we should add an example in the examples folder too, something like this:

const getReasonWhyKidsLoveTheTasteOfCinnamonToastCrunch = require( 'why-do-kids-love-the-taste-of-cinnamon-toast-crunch') const getMilksICanHaveCinnamonToastCrunchWith = getReasonWhyKidsLoveTheTasteOfCinnamonToastCrunch.canIHaveItWithMilk for (const milkICouldHaveItWith of getMilksICanHaveCinnamonToastCrunchWith()) { console.log(`I could have it ${milkICouldHaveItWith}`) }

I don't know what to name this example though LOL. Just make sure you use an underscore (_) between each word, like the other files in the examples folder.

Also, please put a line break above the module.exports line - you can look at the picture below to see how the code you added doesn't quite fit the same style as the other ones, but that's just a simple fix.

blazgocompany · 2024-06-29T18:15:14Z

Ok sorry I didn't see this. Ill work on it.

index.js

blazgocompany · 2024-07-19T21:34:19Z

Hello????

*echoes*
@towerofnix

towerofnix · 2024-07-19T22:11:27Z

Sorry, we're on vacation for some school-related stuff! Check back between one week and fourteen years. 👍

blazgocompany · 2024-07-22T21:01:31Z

I thought Apple would acquire Github by then.

blazgocompany · 2024-07-29T20:52:51Z

Looks like it's been a week... Will I need to wait 14 years or will this be merged @towerofnix?

towerofnix · 2024-07-29T22:37:39Z

Closer to one end of those two, for sure. ☑️

blazgocompany · 2024-07-30T21:32:41Z

@towerofnix · Jun 29

Thanks, this is almost there!! I'm a stickler for literals, so can we try a syntax that's more like...

module.exports.whichMilkCanIHaveItWith =
  function* getMilkCombinations() {
   yield "with 2% milk"
   yield "with whole milk"
   yield "with almond milk"
   yield "with coconut milk"
   yield "with soy milk"
   yield "with cinnamon milk"
   yield "with your favorite milk"
   yield "or even dry"
  }

I also believe that is supposed to be like this with return at the end right?

module.exports.whichMilkCanIHaveItWith =
  function* getMilkCombinations() {
   yield "with 2% milk"
   yield "with whole milk"
   yield "with almond milk"
   yield "with coconut milk"
   yield "with soy milk"
   yield "with cinnamon milk"
   yield "with your favorite milk"
   return "or even dry"
 }

towerofnix · 2024-07-30T22:38:48Z

No, it's different from an iteration perspective.

function* foo() {
    yield 'foo'
    yield 'bar'
    return 'baz'
}

for (const kaz of foo()) {
    console.log(kaz)
}
// 'foo'
// 'bar'
// but NOT 'baz'

blazgocompany · 2024-07-31T14:43:22Z

Oh, I just thought so because of this: https://javascript.info/generators#generator-functions

Generator Functions

To create a generator, we need a special syntax construct: function*, so-called “generator function”.
It looks like this:
function* generateSequence() {
 yield 1;
 yield 2;
 return 3;
}

blazgocompany

@towerofnix I made some changes, and I hope we are ready to merge - We have to! because there's a small mistake!

index.js

blazgocompany · 2024-08-09T14:48:38Z

index.js

+    yield "with cinnamon milk"
+    yield "with your favorite milk"
+    return "or even dry."
+    // console.log("Dry! WHAT! How dare you!")


@towerofnix Let me know if I should remove this line...

Please:

Drop this commented-out line

As discussed in Can I have it with milk? #18 (comment), change return to yield

Get rid of the period at the end, just leave it as "or even dry"

The output value of this function (milk combinations) belongs in the same part of a human sentence every time. We don't need to imply the sequence is over by using .—the consumer of the generator will already understand when the sequence is done. (Technically it's not done after this yield. It's still going to do a final "yield" with {value: undefined, done: true}, as expected!)

towerofnix

Check out our comments, please. The main point is that you should not be bringing unrelated changes into a pull request that is about having it with milk. Use separate branches in your GitHub fork repository (each starting from the current state of the original repository's mistress branch) and file unrelated changes in separate pull requests.

Basically, there is no reason (for instance) to make a gravely essential fix to a public interface's apparent name wait around on perfecting the expression of different combinations with or without milk! That is a fix that can be merged very quickly and likewise published to NPM. But we can't do that if the fix is bundled in with a whole bunch of other unrelated changes that we are taking time to get right. That's why it's important to make separate pull requests from different branches.

There are also a couple of changes requested to do with the milk part of this milky milk milk can I have it with milk pull request. Thanks! 👍

blazgocompany · 2024-08-09T19:39:44Z

oh.. uh oh.... UH OH! Sorry, I'll change it back and create separate PRs.

blazgocompany · 2024-08-09T19:44:45Z

Done!, the only 2 changes I am making is the fix for the name and the milky milk milk milky change

blazgocompany · 2024-08-14T18:59:30Z

@towerofnix Done! Can we merge this milk?

blazgocompany · 2024-08-16T23:08:59Z

@towerofnix MERGE THE MILK!!!!

^{Sorry, I meant merge the PR.}
^{_{If I need to change something please let me know.}}
^{_{^_Ok?}}
^{_{^_Bye!}}

micahlt · 2024-08-17T03:07:11Z

Hi, just a friendly piece of advice @blazgocompany: it isn't polite to spam on a GitHub issue or PR until it gets accepted. @towerofnix is pretty on top of things and most likely busy with life or another project that is significantly more important than this. They will merge this when they get around to it, so let's not fill up their email inbox, all right?

towerofnix · 2024-08-17T16:32:54Z

Thanks @micahlt!

@blazgocompany The reason we haven't merged this yet is that we are meaning to add the example that we mentioned up here #18 (comment) and frankly we don't have the time to back-and-forth on getting it quite right LOL. We'll add it ourselves when we can, but we haven't gotten around to it yet.

We still need you to revert the change on this line https://github.com/towerofnix/why-do-kids-love-the-taste-of-cinnamon-toast-crunch/pull/18/files#r1711611318 — leave it as nobodyToldMeYouThatYouCould like it was before this pull request, and if you want to see it fixed, file a separate pull request. I know it seems trivial, but it really is unrelated to milk, and should not be part of the milk PR.

The rest of the stuff you reverted is good, thanks.

Also make the changes we requested in https://github.com/towerofnix/why-do-kids-love-the-taste-of-cinnamon-toast-crunch/pull/18/files#r1711747374 — I don't believe any of these ones have been addressed yet.

blazgocompany · 2024-08-18T23:31:01Z

@towerofnix, Sorry if my message went a little too far. Ok, I'll work on those. Thank you. I was just wondering if you were just busy or there was something I actually needed to work on.

towerofnix · 2024-08-20T20:32:28Z

It's cool! Yeah those were far but I don't actually mind, and it's clear you were wondering if anything needed to be changed. ("If I need to change something please let me know. Ok? Bye!") FWIW we didn't mark your comments as spam (I don't remember doing that anyway???) apart from locking #19 - hiding your comments would have been GitHub or whatever. No hurt feelings 👍 just best to ask directly if you want to know if there are any changes still needed!!

Speaking of...

towerofnix

...The code changes in this PR look spot-on now! Marking as approved and self-assigning so we come back to it and add the example when we have the moment. Thanks for handling and bringing in all the requested changes!!

blazgocompany · 2024-08-20T21:07:51Z

Yeah, I usually don't like spamming, but sometimes the silence feels a little... awkward.

And aren't the examples supposed to go in another PR? (Sorry if I don't know what goes where)

And by the way.. I hid those myself _{^{hehe.. 😁}}

blazgocompany added 2 commits June 26, 2024 07:21

Update index.js

fb62ba7

Update index.js

0e421a8

towerofnix requested changes Jun 26, 2024

View reviewed changes

Update index.js

a076d1d

towerofnix requested changes Jun 29, 2024

View reviewed changes

Update index.js

5e0c5c2

towerofnix requested changes Jul 3, 2024

View reviewed changes

index.js Show resolved Hide resolved

Update index.js

b4e0aa6

Update index.js

3649a8d

towerofnix self-requested a review July 19, 2024 22:11

Update index.js

6067974

blazgocompany commented Aug 9, 2024

View reviewed changes

Update index.js

9358d47

towerofnix requested changes Aug 9, 2024

View reviewed changes

Update index.js

c269e8d

Update index.js

bcbaacc

blazgocompany added 2 commits August 19, 2024 14:45

Update index.js

f86e946

Update index.js

9589b21

towerofnix approved these changes Aug 20, 2024

View reviewed changes

towerofnix self-assigned this Aug 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Can I have it with milk? #18

Can I have it with milk? #18

blazgocompany commented Jun 26, 2024

towerofnix left a comment

blazgocompany commented Jun 26, 2024

blazgocompany commented Jun 26, 2024

towerofnix Jun 29, 2024

blazgocompany commented Jun 29, 2024

blazgocompany commented Jul 19, 2024

towerofnix commented Jul 19, 2024

blazgocompany commented Jul 22, 2024

blazgocompany commented Jul 29, 2024

towerofnix commented Jul 29, 2024

blazgocompany commented Jul 30, 2024 •

edited

Loading

towerofnix commented Jul 30, 2024

blazgocompany commented Jul 31, 2024 •

edited

Loading

Generator Functions

blazgocompany left a comment

blazgocompany Aug 9, 2024

towerofnix Aug 9, 2024

towerofnix left a comment •

edited

Loading

blazgocompany commented Aug 9, 2024

blazgocompany commented Aug 9, 2024

blazgocompany commented Aug 14, 2024

blazgocompany commented Aug 16, 2024

micahlt commented Aug 17, 2024

towerofnix commented Aug 17, 2024 •

edited

Loading

blazgocompany commented Aug 18, 2024 •

edited

Loading

towerofnix commented Aug 20, 2024

towerofnix left a comment

blazgocompany commented Aug 20, 2024 •

edited

Loading

Can I have it with milk? #18

Are you sure you want to change the base?

Can I have it with milk? #18

Conversation

blazgocompany commented Jun 26, 2024

towerofnix left a comment

Choose a reason for hiding this comment

blazgocompany commented Jun 26, 2024

blazgocompany commented Jun 26, 2024

towerofnix Jun 29, 2024

Choose a reason for hiding this comment

blazgocompany commented Jun 29, 2024

blazgocompany commented Jul 19, 2024

Hello????

towerofnix commented Jul 19, 2024

blazgocompany commented Jul 22, 2024

blazgocompany commented Jul 29, 2024

towerofnix commented Jul 29, 2024

blazgocompany commented Jul 30, 2024 • edited Loading

towerofnix commented Jul 30, 2024

blazgocompany commented Jul 31, 2024 • edited Loading

Generator Functions

blazgocompany left a comment

Choose a reason for hiding this comment

blazgocompany Aug 9, 2024

Choose a reason for hiding this comment

towerofnix Aug 9, 2024

Choose a reason for hiding this comment

towerofnix left a comment • edited Loading

Choose a reason for hiding this comment

blazgocompany commented Aug 9, 2024

blazgocompany commented Aug 9, 2024

blazgocompany commented Aug 14, 2024

blazgocompany commented Aug 16, 2024

@towerofnix MERGE THE MILK!!!!

micahlt commented Aug 17, 2024

towerofnix commented Aug 17, 2024 • edited Loading

blazgocompany commented Aug 18, 2024 • edited Loading

towerofnix commented Aug 20, 2024

towerofnix left a comment

Choose a reason for hiding this comment

blazgocompany commented Aug 20, 2024 • edited Loading

blazgocompany commented Jul 30, 2024 •

edited

Loading

blazgocompany commented Jul 31, 2024 •

edited

Loading

towerofnix left a comment •

edited

Loading

towerofnix commented Aug 17, 2024 •

edited

Loading

blazgocompany commented Aug 18, 2024 •

edited

Loading

blazgocompany commented Aug 20, 2024 •

edited

Loading