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

Add warning suppression functionality #2911

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

owenatgov
Copy link
Contributor

Fixes #2890

This PR does the following:

  • Adds the $govuk-suppressed-warnings map
  • Adds the _warning mixin
  • Replaces all instances of @warn with the _warning mixin
  • Updates the managing-change docs with why we allow warning suppression

The settings/warnings suite ($govuk-suppressed-warnings and _warning) are intended as a way to allow users to suppress deprecation warnings in instances where they're stuck on specific versions so they have the choice to keep their sass compile log clean. It also will allow us to suppress the compatibility mode deprecation warnings in our local legacy app between now and the release of v5.0.

More details are in the commit history.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 10, 2022 17:52 Inactive
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from 8830e49 to 614fe69 Compare October 10, 2022 17:52
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 10, 2022 17:52 Inactive
@owenatgov owenatgov marked this pull request as ready for review October 10, 2022 17:53
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from 614fe69 to cb2004a Compare October 10, 2022 17:55
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 10, 2022 17:55 Inactive
@owenatgov owenatgov requested a review from a team October 10, 2022 17:55
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from cb2004a to 6bb713b Compare October 10, 2022 17:59
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 10, 2022 17:59 Inactive
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Looking pretty much there for me 🙌🏻 Just noticed we're missing a test case and nitpicked on the terminology for the $govuk-suppressed-warnings map 😊

src/govuk/settings/warnings.test.js Show resolved Hide resolved
src/govuk/settings/_warnings.scss Outdated Show resolved Hide resolved
src/govuk/settings/_warnings.scss Outdated Show resolved Hide resolved
src/govuk/settings/_warnings.scss Outdated Show resolved Hide resolved
docs/contributing/managing-change.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from 6bb713b to c4f7c96 Compare October 11, 2022 14:44
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 11, 2022 14:45 Inactive
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from c4f7c96 to 062c4f9 Compare October 13, 2022 10:29
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 13, 2022 10:29 Inactive
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from 062c4f9 to 07f1181 Compare October 13, 2022 10:30
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 13, 2022 10:30 Inactive
@querkmachine
Copy link
Member

Code-wise this is looking pretty cushy to me, but I'll leave the formal review for the folks who've been looking at it for longer. 😉

@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from 07f1181 to c181db4 Compare October 13, 2022 10:54
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 13, 2022 10:55 Inactive
These are a setting to record which sass warnings users suppressed and a means to interact with said settings respectively.
Specific changes:

- Replace all instances of `@warn` with the `_warning` mixin
- Add a section explaining the `_warning` mixin and warning suppression
@owenatgov owenatgov force-pushed the warning-suppression-functionality branch from c181db4 to 26611f5 Compare October 13, 2022 16:05
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2911 October 13, 2022 16:05 Inactive
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Thanks for updating the tests and docs 🙌🏻

@owenatgov
Copy link
Contributor Author

With 2 approvals I'm gonna go ahead and merge this. There was a slight change since Claire's approval to the managing change docs but this is intended as internal content and can be changed at any time so I'm not worried about this.

@owenatgov owenatgov merged commit b082be7 into main Oct 14, 2022
@owenatgov owenatgov deleted the warning-suppression-functionality branch October 14, 2022 09:31
@romaricpascal romaricpascal mentioned this pull request Nov 10, 2022
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.

Allow users to suppress deprecation warnings
6 participants