These are chat archives for ericelliott/stampit

9th
Jun 2015
Carl Olsen
@unstoppablecarl
Jun 09 2015 10:59
@koresar maybe it is just me but i am finding it increasingly hard to follow the mixer code as written.
Vasyl Boroviak
@koresar
Jun 09 2015 11:00
+1
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:00
Vasyl Boroviak
@koresar
Jun 09 2015 11:00
But if you try to read lodash code its even harder. Isn't it?
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:01
yes some of it
Vasyl Boroviak
@koresar
Jun 09 2015 11:01
No, it doesn't. "target" can replace it. Why?
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:02
I think it would be more clear
and probably faster without a context but I am not 100% sure about that
also what is this about?
Vasyl Boroviak
@koresar
Jun 09 2015 11:04
Bloody hell! That's a leftover code. Pardon. I'll remove it.
I'll change "this" to "target".
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:07
return function mixIn(target) {
  var loop = opts.chain ? forIn : forOwn;
  var i = 0,
    n = arguments.length,
    filter = opts.filter,
    getValue = opts.getValue,
    obj;
  target = opts.getTarget ? opts.getTarget(target) : target;

  var mergeValue = function mergeValue(val, key) {
    var targetVal = target[key];
    if (filter && !filter(val, targetVal)) {
      return;
    }

    if(getValue){
      targetVal = getValue(val, targetVal);
    }
    target[key] = targetVal;

  };

  while (++i < n) {
    obj = arguments[i];
    if (obj) {
      loop(
        obj,
        mergeValue,
        target);
    }
  }
  return target;
};
I think this is much easier to follow what do you think?
Vasyl Boroviak
@koresar
Jun 09 2015 11:08
Agree.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:08
too many opts. adds to cognitive load for me
Vasyl Boroviak
@koresar
Jun 09 2015 11:09
Could you add the comments to the PR appropriate lines?
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:09
and this would not need the target param in loop right?
comments?
Vasyl Boroviak
@koresar
Jun 09 2015 11:09
Right.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:09
you mean commits?
Vasyl Boroviak
@koresar
Jun 09 2015 11:10
You can comment each individual line in a PR. Click plus on the left.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:10
I am a contributor now I can just commit to the branch
Vasyl Boroviak
@koresar
Jun 09 2015 11:11
Please, do not commit to other people PR's. That confuses a lot.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:11
ok
you want me to just make comments with my suggestions?
Vasyl Boroviak
@koresar
Jun 09 2015 11:11
Exactly! This what we call "review".
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:12
ok I wasn’t sure what you mean by the appropriate lines lol
Vasyl Boroviak
@koresar
Jun 09 2015 11:12
Oops
I meant "corresponding". Sorry.
I speak 3 languages. That's hard for my math brains.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:13
is there a good way to mark comments on your pr code but be able to see all of the code not just the diff view?
your english is very good
Vasyl Boroviak
@koresar
Jun 09 2015 11:14
You can click "expand" near each diff.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:15
I never noticed that thanks
hum but I can’t put notes on those expanded lines :(
ok I added them is this what you wanted?
@koresar ?
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:20
also the word inception does not mean what you think it means
Vasyl Boroviak
@koresar
Jun 09 2015 11:26
I copied it from the "mout" module stampit v1 is using.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:27
ok
so when creating an instance props will not overwrite existing properties?
that seems weird
Vasyl Boroviak
@koresar
Jun 09 2015 11:30
Many things in JS seem weird.
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:32
personally I would rather the object used in the creation of an instance to be passed to init, so you can decide what you want to do with it yourself
what do you think about that?
Vasyl Boroviak
@koresar
Jun 09 2015 11:37
"I would rather" what?
Wilfried Boukhers
@Bear-Foot
Jun 09 2015 11:37
( the object used in the creation of an instance)
To be passed to init
Vasyl Boroviak
@koresar
Jun 09 2015 11:40
stamp(refs, arg1) - the arg1 will be passed to init()
No, we are not changing the current API
Carl Olsen
@unstoppablecarl
Jun 09 2015 11:46
@koresar was props not overwriting exisitng properties discussed? where?
Vasyl Boroviak
@koresar
Jun 09 2015 11:51
It was not.
Vasyl Boroviak
@koresar
Jun 09 2015 12:01
@unstoppablecarl next time click "Files changed" and review the combined changes: https://github.com/ericelliott/stampit/pull/96/files
Please, avoid commenting each individual commit.
thanks
Carl Olsen
@unstoppablecarl
Jun 09 2015 12:51
ok cool
Tim Routowicz
@troutowicz
Jun 09 2015 23:06
hey guys o/
Vasyl Boroviak
@koresar
Jun 09 2015 23:13
Oi mate
Tim Routowicz
@troutowicz
Jun 09 2015 23:16
how are you @koresar
Vasyl Boroviak
@koresar
Jun 09 2015 23:17
I'm very good. How are your things Tim?
Tim Routowicz
@troutowicz
Jun 09 2015 23:17
things are good Vasyl ;)
im going to start ES6 conversion soon, just been finishing up things with react-stampit
Vasyl Boroviak
@koresar
Jun 09 2015 23:45
Wait for the last PR merged and then start your ES6 branch.