Remove unneeded settings import #752
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As all the settings are `!default`, the expected way to override settings is to define them before doing the import. Importing the settings _generally_ works, because we just end up redefining them before they get used. However, it does break some things – specifically, where settings rely on each other (such as the legacy mode settings, or $govuk-assets-path which is relied on by $govuk-images-path and $govuk-fonts-path). This doesn't work correctly because e.g. $govuk-images-path gets defined before $govuk-assets-path gets overridden, which means it uses the _old_ assets path. This caught out a user who was using this as a template, and was overriding $govuk-assets-path but not seeing the expected changes represented in $govuk-images-path.
|
@NickColley just wanted to check if you're happy with this change, as I think you added this line originally? |
NickColley
approved these changes
May 28, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As all the settings are
!default, the expected way to override settings is to define them before doing the import.Importing the settings generally works, because we just end up redefining them before they get used. However, it does break some things – specifically, where settings rely on each other (such as the legacy mode settings, or $govuk-assets-path which is relied on by $govuk-images-path and $govuk-fonts-path).
This doesn't work correctly because e.g. $govuk-images-path gets defined before $govuk-assets-path gets overridden, which means it uses the old assets path.
This caught out a user who was using this as a template, and was overriding $govuk-assets-path but not seeing the expected changes represented in $govuk-images-path.