Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update Clockface Fonts #755

Merged
merged 7 commits into from
Apr 21, 2022
Merged

Update Clockface Fonts #755

merged 7 commits into from
Apr 21, 2022

Conversation

brandenTenbrink
Copy link
Contributor

@brandenTenbrink brandenTenbrink commented Apr 8, 2022

Closes #

Changes

Updated the Clockface fonts:

Code Font: IBMPlexMono => Roboto Mono

Text Font: Rubik => ProximaNova

Screenshots

// Add screenshots here if relevant
Screen Shot 2022-04-08 at 1 28 35 AM

Screen Shot 2022-04-08 at 1 28 43 AM

Checklist

Check all that apply

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved

@brandenTenbrink
Copy link
Contributor Author

@hoorayimhelping Figured we could try and avoid a meeting if you just wanna check the webpack stuff here async. I think its right since everything is loading fine.

Copy link
Contributor

@hoorayimhelping hoorayimhelping left a comment

Choose a reason for hiding this comment

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

Looks good to me, not counting the consideration on versioning. The webpack changes look fine - doesn't seem like a big deal, just adding in support for more font formats.

A consideration: should this count as a breaking change? Would a hypothetical (or actual) consumer of the Clockface library need to change their application in response to pulling in this change? Could this change potentially break an application using Clockface as its component library? @mavarius spoke to me about needing to QA these changes before moving them to prod. To me that implies that things could break if a user pulls in these changes; I think that implies we should bump Clockface up to version 4.0.0 with this change.

Semver (which is the x.y.z versioning scheme we use in libraries) for reference. This is one of those cases where it'll take some human judgement to determine what version this is, but I'm leaning towards bumping Clockface up to 4.0.0 since changes in fonts and headings might cause cascading CSS changes in an application.

@brandenTenbrink
Copy link
Contributor Author

Looks good to me, not counting the consideration on versioning. The webpack changes look fine - doesn't seem like a big deal, just adding in support for more font formats.

A consideration: should this count as a breaking change? Would a hypothetical (or actual) consumer of the Clockface library need to change their application in response to pulling in this change? Could this change potentially break an application using Clockface as its component library? @mavarius spoke to me about needing to QA these changes before moving them to prod. To me that implies that things could break if a user pulls in these changes; I think that implies we should bump Clockface up to version 4.0.0 with this change.

Semver (which is the x.y.z versioning scheme we use in libraries) for reference. This is one of those cases where it'll take some human judgement to determine what version this is, but I'm leaning towards bumping Clockface up to 4.0.0 since changes in fonts and headings might cause cascading CSS changes in an application.

Yeah, I agree on 4.0.0 for this change. There are several updates going out between now and next week, so I will be bumping the version once I know their order.

Copy link
Collaborator

@mavarius mavarius left a comment

Choose a reason for hiding this comment

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

Looks good!

@brandenTenbrink brandenTenbrink merged commit 741d321 into master Apr 21, 2022
@brandenTenbrink brandenTenbrink deleted the feat/update_clockace_fonts branch April 21, 2022 18:43
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants