Skip to content
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

Document configuring through imports #425

Merged
merged 2 commits into from
Dec 26, 2019
Merged

Document configuring through imports #425

merged 2 commits into from
Dec 26, 2019

Conversation

jathak
Copy link
Member

@jathak jathak commented Dec 26, 2019

No description provided.

@jathak jathak requested a review from nex3 December 26, 2019 20:08
@nex3 nex3 temporarily deployed to sass-lang-pr-425 December 26, 2019 20:08 Inactive

<% impl_status dart: '1.24.0', libsass: false, ruby: false %>

You can configure modules that are loaded through an `@import` simply by
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to avoid words like "just" and "simply"—they can feel patronizing for some readers.

Also, link "configure modules" to the section describing how default variables work. Users reading this may not have the context to understand exactly what kind of "configuration" you're referring to.

defining global variables prior the `@import` that first loads that module.

Note that changes to the configuration after the module is first loaded will be
ignored.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this kind of confusing. Either elaborate on it by describing exactly the situation in which this might be a problem (possibly within a <% heads_up do %> block), or leave it out if you don't think it's likely to be a substantial problem in practice.

Comment on lines 558 to 559
$lib-color: red;
@import "library";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is just going to produce more net confusion. Examples on the doc site don't have to be 100% comprehensive (that's what the spec is for), so it's more important that they show the most focused version of a feature possible.

@jathak jathak requested a review from nex3 December 26, 2019 20:38
@nex3 nex3 temporarily deployed to sass-lang-pr-425 December 26, 2019 20:38 Inactive
Comment on lines 576 to 578
Modules are only loaded once, so if you change the configuration after you
`@import` a module for the first time (even indirectly), the change will be
ignored if you `@import` the module again.
Copy link
Contributor

@nex3 nex3 Dec 26, 2019

Choose a reason for hiding this comment

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

Nit: indent this two spaces.

@jathak jathak merged commit d023f81 into master Dec 26, 2019
@jathak jathak deleted the import-configuration branch December 26, 2019 21:00
asaf400 pushed a commit to asaf400/ass-site that referenced this pull request Apr 18, 2024
asaf400 pushed a commit to asaf400/ass-site that referenced this pull request Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants