-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update link to Polaris v3.10.0 #304
Conversation
@@ -92,6 +92,25 @@ test('it renders the correct HTML with external attribute', function(assert) { | |||
); | |||
}); | |||
|
|||
test('it renders the correct HTML with monochrome attribute', function(assert) { | |||
this.render(hbs` |
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.
would it be too much to ask to convert the test to new syntax? 😬
we can leave for later too, but would be nice to do it while here
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.
I'd prefer to get the update done in one go as quickly as possible so we can start using it, and update tests en masse in the future - they're separate changes in my mind.
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.
as you think it's best (anything needed for the rush?)
from my experience, that en masse = never lmao
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.
anything needed for the rush?
Updating the Polaris version takes a long time and we'll almost certainly have other projects coming up which will divert resources away from the update, and potentially add new parts of our apps that need updating. Test syntax updates can be done either in bulk or on an individual file basis moving forward so they're less "intrusive" changes, and don't really add any value as they're more of a "sideways" move. Getting the update out and in use seems like the more pressing change at this point 🤷♂️
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.
good with me
though, again, that's the reason we kicked of this update.... nothing should be pressing
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.
Maybe my wording oversold the urgency of this 😂 But I still feel like a better approach is to get the code changes required for the Polaris update out of the way first in one go, then make further updates as and when we have the cycles to spare 🙏
Adds support for
monochrome
polaris-link
s and ensures the correcttype
is used when link buttons are rendered.