Fix: organize prop & attr code to be similar #2427
Conversation
This reverts commit d0388e9.
pgilad
changed the title
|
/cc @gibson042, since you reviewed pr to |
There was a problem hiding this comment.
This existed in the code before my previous commit. See
Line 6 in 0de798d
There was a problem hiding this comment.
It doesn't exist in current compat:
Line 6 in d0388e9
There was a problem hiding this comment.
Yes because that was a bad merge from master by yours truly 😊
There was a problem hiding this comment.
I was thinking to unite those commits so revert one would go away and from history stand point, it would be just support commit
Now we're commenting on a Frankenstein diff...
nicely put, lol
There was a problem hiding this comment.
IMO it's harder to see the logic of the change when it's modifying the previous bad merge.
But however your prefer, just my opinion.
There was a problem hiding this comment.
We can compare it like that https://github.com/pgilad/jquery/compare/6df1bf9be818b8955f94113aecc689fdf7a2e172...ab692d396542b1a65a93460359fe1d8c6ca6ccf6
Although, i'm also not pushing for specific land strategy
There was a problem hiding this comment.
I wonder, we don't have it in master plus it seems we don't use this declaration, test doesn't fail without it. Maybe it is needed to build this module and that is indirect dependency?
@timmywil? We can deal with this later though
There was a problem hiding this comment.
My guess is it's leftover from a prop hook for retrieving button value or something. It doesn't seem necessary anymore.
|
I would like to merge it, waiting couple hours for @gibson042 response |
|
Hey @markelog thanks for the review. I added some extra blank spaces where needed. |
src/attributes/attr.js
Outdated
There was a problem hiding this comment.
Would love to change this to simple if...else, but i guess this story for another day
|
Closed via f2bcf87 |
|
It seems we're passed on first try, thank you! |
|
👏 |
5 participants
This fixes the previous faulty commit of d0388e9 on the
compatbranch.The changes of that commit were reverted and the new (hopefully) correct changes were added again.