-
Notifications
You must be signed in to change notification settings - Fork 204
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: remove redundant form-control in masquerade user input #1416
fix: remove redundant form-control in masquerade user input #1416
Conversation
Thanks for the pull request, @tecoholic! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:
When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1416 +/- ##
=======================================
Coverage 88.83% 88.83%
=======================================
Files 307 307
Lines 5268 5268
Branches 1338 1338
=======================================
Hits 4680 4680
Misses 572 572
Partials 16 16 ☔ View full report in Codecov by Sentry. |
@tecoholic Is this waiting for OpenCraft internal review or did you want to send it straight to upstream review? |
@itsjeyd I have pinged @farhaanbukhsh internally to take a look at this. However, I wasn't certain if he has CC access to this repo at that time. The CC list indicates there are no CCs from OpenCraft who have access. This PR was a side effect of an internal ticket and, as such, doesn't fix anything. So, I would like to get an upstream review directly if possible. |
Hey @tecoholic I am not a CC/maintainer for the repo :) from backstage I can see that 2u-aurora maintains it. |
@tecoholic Got it, I'll mark this as ready for review now. @farhaanbukhsh @tecoholic The Aurora team is stepping down as maintainer and most likely won't be reviewing this PR. @feanil @arbrandes @brian-smith-tcril Would one of you be able to help us get this tiny change over the line? |
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.
Change looks good and tested locally just fine.
@tecoholic 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Thanks a lot @feanil. |
Description
This PR just removes a redundant class from the code. There are no UI changes in the master version as the same class is applied to twice. This was causing an issue in the previous versions which are using the
@edx/paragon
andFormControl
in theMasqueradeUserNameInput
(Ref: see open-craft#24).Since master branch uses
@openedx/paragon
andInput
in theMasqueradeUserNameInput
component, the styles don't really cause any UI issues and it is just a minor improvement in code quality.Changes
Before
After