Number: Refactor formatter to extract lots of variables#484
Number: Refactor formatter to extract lots of variables#484jzaefferer wants to merge 1 commit intoglobalizejs:masterfrom
Conversation
|
@jzaefferer thanks for your change. Although I'm not against the change, I didn't understand its value? All variables are still there, although they have been moved into a different function. |
|
It makes the formatter function easier to understand, since there's a lot less variables in its scope. |
|
Seems good. |
jzaefferer
force-pushed
the
mini-validate-refactor
branch
from
2578089 to
a88793d
Compare
|
Rebased. |
There was a problem hiding this comment.
Coding style now allows onevar: false, which I personally like too. Although, the rest of the code still uses onevar: true. For consistency sake, I suggest this to be updated to onevar: true until #413 lands.
There was a problem hiding this comment.
We've been doing gradual migrations elsewhere. I see no point introducing new code with the old style just for consistency. Anything new shouldn't use onevar is easy enough to apply.
There was a problem hiding this comment.
There was a problem hiding this comment.
Well, the examples are from different projects. I wouldn't encourage having inconsistent code.
If it helps, feel free to cherry-pick rxaviers@26f7e10.
There was a problem hiding this comment.
I don't really mind, feel free to land this with your fixup commit.
|
I left one comment above. Other than that, let's get it landed. Thanks. |
4 participants
Ref #436 (comment)