-
Notifications
You must be signed in to change notification settings - Fork 551
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
Fix #1882:Shifted NumericInput Rules test to numberinput subpackage #1909
Conversation
@rt4914 @BenHenning PTAL |
@soamOne Thanks for creating this PR.
|
Hey @Sarthak2601 @rt4914 @BenHenning PTAL now. |
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.
LGTM. Thanks @soamOne !
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.
LGTM, thanks.
Thanks |
Thanks @soamOne. These files ought to go under |
@BenHenning I will look into it and fix it, thanks. |
)" This reverts commit 8f34df0.
Also I just realized that the original package was never updated to match the new directory structure. This is actually a case where Gradle misbehaves: it (incorrectly) doesn't require directory structures to match their directory structures, but Bazel does. Given that the build isn't actually broken, I won't revert this PR, but we should prioritize fixing that since it can lead to confusion. Thanks @soamOne! |
Okay, so what all needs to be done now @BenHenning ? |
@soamOne see #1882 (comment). Does that help? |
Yes @BenHenning, got it. |
Hey @BenHenning, a couple of questions:
|
|
@rt4914 okay, got it. |
Explanation
Fix #1882: In this PR we are shifting numeric input rules tests to a new subpackage called numberinput.
Checklist
@rt4914 @BenHenning