-
Notifications
You must be signed in to change notification settings - Fork 15
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
add unary operators 'lowercase' and 'uppercase' #541
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Great, thanks! Please first sign the CLA (commenting the sentence the bot asks you to, then retriggering the check). Then you can add me as a reviewer (top right) and I'll get a notification to review this asap :). |
bf7b4c5
to
716c17e
Compare
I have read the CLA Document and I hereby sign the CLA |
Looks good, now last thing left is officially requesting a review from me (top right, reviewers) and then I'll try to get to it asap. If you have time left in the meantime, you can familiarize yourself with the type system a bit by trying to understand if we can allow string and regex values for a parameter (for this task #523). |
Hey @TungstnBallon, I merged some changes in #539 that affect your changes. I suggest doing a Let me know if you need help with that ;-) |
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.
Looks good to me 👍 .
Regarding review flow, what I do is:
- Approve with comments: You can merge at any time, fixing the comments would be good but I'll leave it up to your best judgement how / if it makes sense.
- Request changes: Don't merge before the comments are resolved. Once you consider the comments resolved, re-request a new review from the PR main page.
So in this case, you can use your best judgement if you want to accept my suggested changes or not and afterwards you are free to merge without a new review.
716c17e
to
a41762e
Compare
adds the
lowercase
andlowercase
operators.I adapted `cars.jv* to demonstrate them. This file adds 2 new columns, one for the lowercase and one for the uppercase name.
closes #540
@rhazn