-
Notifications
You must be signed in to change notification settings - Fork 128
feat: import FooterSlot
from component package instead of slot package
#604
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: import FooterSlot
from component package instead of slot package
#604
Conversation
d657ef7
to
b64d57b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #604 +/- ##
=======================================
Coverage 97.43% 97.43%
=======================================
Files 152 152
Lines 1327 1327
Branches 219 220 +1
=======================================
Hits 1293 1293
Misses 33 33
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sandbox deployment successful 🚀 |
b64d57b
to
de72055
Compare
Sandbox deployment successful 🚀 |
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.
blocked until edx/frontend-component-footer-edx#476 is resolved.
@deborahgu can someone address edx/frontend-component-footer-edx#476 ASAP? The plan is to land these for Teak and the cut is on Thursday. |
@deborahgu What's the time frame for landing the workaround? We think this is a simple fix, includes expand contract for y'all, and we need to land this for the Teak cut-off, Thursday. |
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.
The PR itself looks good. 👍🏼
I'm investigating and will note back as soon as I hear anything. No change is that simple when it's an upstream component and the change has to propagate through multiple repos owned by multiple stakeholders. 😉 |
@deborahgu I do want to emphasize that the change I suggested in edx/frontend-component-footer-edx#476 is non-breaking. |
Thanks, we'll look at the change. Since we learned about the breaking changes after the end of day Thursday, and it's been a long weekend, this is giving us fewer than three days in our normal workflow to see what we need to change on our end. We are treating this as a top priority, but please understand that this is an extraordinary ask and we're not going to be able to validate this in a five minute turnaround. We still intend to land it before teak. |
@deborahgu We really appreciate you prioritizing it. It's incredibly helpful. We -- @brian-smith-tcril mostly -- have tried to make it a seamless change with a shim to provide backward compatibility for y'all. A little further along, we should have a retro. There are some key areas to discuss in the process I think. |
de72055
to
a1f5e49
Compare
https://app.pr-604-550b00.sandboxes.opencraft.hosting/learner-dashboard/