Skip to content

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 22, 2021

Adds a custom stylelint rule that will prevent multi-line comments (/* */) from being used in .scss files. The problem with such comments is that they are preserved by Sass and will show up in the user's output, whereas single-line comments (//) will not.

Note: while Stylelint has a comment-pattern rule already, we can't use it because the rule works on the comment text which doesn't include the syntax.

…ed output

Adds a custom stylelint rule that will prevent multi-line comments (`/* */`) from being used in .scss files. The problem with such comments are preserved by Sass and will show up in the user's output, whereas single-line comments (`//`) will not.

**Note:** while Stylelint has a `comment-pattern` rule already, we can't use it because the rule works on the comment text which doesn't include the syntax.
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent merge safe target: major This PR is targeted for the next major release labels Mar 22, 2021
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 22, 2021
Copy link
Contributor

@andrewseguin andrewseguin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For comments that are meant akin to JsDocs - should we switch to using the /// standard?

https://sass-lang.com/documentation/syntax/comments (see bottom)

@crisbeto
Copy link
Member Author

I think so, but we aren't generating any docs from these comments right now so it doesn't really matter.

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I was planning to do a pass sometime in the near future to change function/mixin/variable descriptions over to SassDoc format

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Mar 22, 2021
@andrewseguin andrewseguin merged commit 3c975e9 into angular:master Mar 23, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants