-
Notifications
You must be signed in to change notification settings - Fork 2k
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 a guideline for mixins #9016
Conversation
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've made some suggestions to frame this more as imperative ("do this") rather than as a fact, since it's still aspirational. 😄
I've used the suggestions tool to propose changes, but I don't need you to apply them directly (in fact, they may fail the linter), especially if you have a better wording in mind.
Co-authored-by: Daniel D. Beck <daniel@ddbeck.com>
Looks like applying the changes worked just fine :) The diff looks funky to me after the code block, no idea why, though. I agree to merge this in the same release as the other PR! |
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.
Is today my birthday? No? Then why am I getting what I've wished for for so long? Thanks @Elchi3!
Co-authored-by: Philip Jägenstedt <philip@foolip.org>
Chris says "I also agree with the BCD approach outlined in #8929". I would say we can merge this. |
Let's merge this such that it lands in the same release as #8933 (I'll try to review that today, otherwise, first thing tomorrow). |
It looks like there is consensus in #8929 so here's a PR that adds a data guideline for mixins.