Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Add Scottish colour and scotland service to fonts #2120

Merged
merged 5 commits into from
Sep 16, 2019
Merged

Conversation

jimjohnsonrollings
Copy link
Contributor

@jimjohnsonrollings jimjohnsonrollings commented Sep 13, 2019

Resolves #2121

Overall change:
Adds the 'scotland' service to font-families, and the dark saltire colour.

Code changes:

  • Add the 'scotland' service to font-families
  • Add the 'dark saltire' colour
  • Update colour unit test

  • I have assigned myself to this PR and the corresponding issues
  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@sareh
Copy link
Contributor

sareh commented Sep 13, 2019

Looks great so far. You'll want to update the package version in the packages/utilities/psammead-styles/package.json and package-lock.json and add a new row at the top of the CHANGELOG.md file. It will be a minor increase since you're adding new exports, but the package as a whole has backward compatibility. (Right now, version 2.3.0)

Copy link
Contributor

@sareh sareh left a comment

Choose a reason for hiding this comment

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

👍 Looks great.

The reason Code Climate is erroring, even though you have added a test for the colour being added, is that none of the unit tests for colours.js are currently sufficient. I have opened an issue here to fix the colours unit tests: #2140 . I think that this PR should not need to wait until that is done to be merged in.

@staylos92
Copy link
Contributor

Happy for this to be merged 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Scotland service to fonts and their colour
5 participants