-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(data-point-codemods): Add codemod for changing PathReducer from $. to $ #183
feat(data-point-codemods): Add codemod for changing PathReducer from $. to $ #183
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so cool!
…ssing-root-path to get 100% cov re ViacomInc#179
…/data-point into add-codemod-for-pathreducer
'transform:d': '$', | ||
'transform:e': '$ | foo', | ||
'transform:f': '$ | $ | $', | ||
'transform:g': 'foo | bar | baz' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have any tests where the $.
is nested inside some other kind of reducer?
// just for example
{
a: ['$.']
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I can add that. Also something like dataPoint.transform('$. | foo', input)
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't hurt
…-path-reducer-accessing-root-pa re: ViacomInc#179
…/data-point into add-codemod-for-pathreducer
} | ||
} | ||
|
||
dataPoint.transform('foo | $. | baz | $.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raingerber Covering a few more scenarios.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow. I overlooked this bug, pushed it, and then commented on it, and I still didn’t notice until the CI failed. 🤦♂️
What: Adds a codemod to change
$.
to$
if found in a string or template literalWhy: Closes #179
How: http://http://astexplorer.net
Checklist: