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.

Sign up for GitHub

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

Jump to bottom

Renames "Fonts Library" to "Font Library" #53780

Merged
merged 5 commits into from
Aug 17, 2023
Merged

Renames "Fonts Library" to "Font Library" #53780

merged 5 commits into from
Aug 17, 2023

Conversation

hellofromtonya
Copy link
Contributor

Part of #53778.

What?

Renames the PHP code from "Fonts Library" (plural) to "Font Library (singular).

Why?

See the reasoning #53778.

How?

Renames the files, directories, comments, functions, and classes to the singular version.

Testing Instructions

All CI jobs should pass ✅

Locally running the test suite should pass:

npm run test:unit:php

@hellofromtonya hellofromtonya added [Type] Enhancement A suggestion for improvement. [Feature] Typography Font and typography-related issues and PRs labels Aug 17, 2023
@hellofromtonya hellofromtonya self-assigned this Aug 17, 2023
@github-actions
Copy link

github-actions bot commented Aug 17, 2023 •
edited
Loading

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ phpunit/fonts/font-library/wpFontLibrary/getFontsDir-test.php
❔ lib/experimental/fonts/font-face/bc-layer/webfonts-deprecations.php
❔ lib/load.php
❔ phpunit/multisite.xml
❔ lib/experimental/fonts/font-library/class-wp-font-family-utils.php
❔ lib/experimental/fonts/font-library/class-wp-font-family.php
❔ lib/experimental/fonts/font-library/class-wp-font-library.php
❔ lib/experimental/fonts/font-library/class-wp-rest-font-library-controller.php
❔ lib/experimental/fonts/font-library/font-library.php
❔ phpunit/fonts/font-library/class-wp-rest-font-library-controller-test.php
❔ phpunit/fonts/font-library/wpFontFamily/__construct-test.php
❔ phpunit/fonts/font-library/wpFontFamily/base.php
❔ phpunit/fonts/font-library/wpFontFamily/getData-test.php
❔ phpunit/fonts/font-library/wpFontFamily/getDataAsJson-test.php
❔ phpunit/fonts/font-library/wpFontFamily/getFontPost-test.php
❔ phpunit/fonts/font-library/wpFontFamily/hasFontFaces-test.php
❔ phpunit/fonts/font-library/wpFontFamily/install-test.php
❔ phpunit/fonts/font-library/wpFontFamily/uninstall-test.php
❔ phpunit/fonts/font-library/wpFontFamilyUtils/getFilenameFromFontFace-test.php
❔ phpunit/fonts/font-library/wpFontFamilyUtils/hasFontMimeType-test.php
❔ phpunit/fonts/font-library/wpFontFamilyUtils/mergeFontsData-test.php
❔ phpunit/fonts/font-library/wpFontLibrary/setUploadDir-test.php

@hellofromtonya hellofromtonya requested review from matiasbenedetto and removed request for spacedmonkey August 17, 2023 18:50
@hellofromtonya
Copy link
Contributor Author

I created this PR ahead of consensus to do the renaming. Once there's consensus, then this PR should be ready to go.

matiasbenedetto
matiasbenedetto reviewed Aug 17, 2023
View reviewed changes
Copy link
Contributor

@matiasbenedetto matiasbenedetto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Could we use the new name here instead of both?

	( defined( 'FONT_LIBRARY_ENABLE' ) && FONT_LIBRARY_ENABLE ) ||
	( defined( 'FONTS_LIBRARY_ENABLE' ) && FONTS_LIBRARY_ENABLE )

hellofromtonya
hellofromtonya commented Aug 17, 2023
View reviewed changes
lib/load.php
Comment on lines +162 to +165
if (
( defined( 'FONT_LIBRARY_ENABLE' ) && FONT_LIBRARY_ENABLE ) ||
( defined( 'FONTS_LIBRARY_ENABLE' ) && FONTS_LIBRARY_ENABLE )
) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matiasbenedetto I left both constants in just in case someone is working on a PR or testing locally that is using the plural version.

This code is temporary. Once the frontend PR lands, then it can be removed.

So I think it's okay to leave it, i.e. just in case to help with developer workflow.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense 👍

matiasbenedetto
matiasbenedetto approved these changes Aug 17, 2023
View reviewed changes
Copy link
Contributor

@matiasbenedetto matiasbenedetto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The name change is OK to me and tests are running as expected, so LGTM 🚀

@hellofromtonya hellofromtonya enabled auto-merge (squash) August 17, 2023 19:37
@hellofromtonya hellofromtonya merged commit 0eebaf5 into trunk Aug 17, 2023
49 checks passed
@hellofromtonya hellofromtonya deleted the try/rename-font-library branch August 17, 2023 19:46
@hellofromtonya hellofromtonya mentioned this pull request Aug 17, 2023
Closed
16 tasks
@github-actions github-actions bot added this to the Gutenberg 16.6 milestone Aug 17, 2023
@mikachan mikachan linked an issue Aug 18, 2023 that may be closed by this pull request
Rename "Fonts Library" to "Font Library" #53778
Closed
@mikachan mikachan mentioned this pull request Sep 5, 2023
Closed
86 tasks
@mikachan mikachan added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@youknowriad youknowriad added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Needs PHP backport Needs PHP backport to Core labels Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matiasbenedetto matiasbenedetto matiasbenedetto approved these changes

Assignees

@hellofromtonya hellofromtonya

Labels
Backported to WP Core Pull request that has been successfully merged into WP Core [Feature] Typography Font and typography-related issues and PRs [Type] Enhancement A suggestion for improvement.
Projects
None yet
Milestone
Gutenberg 16.6
Development

Successfully merging this pull request may close these issues.

Rename "Fonts Library" to "Font Library"
4 participants
@hellofromtonya @matiasbenedetto @youknowriad @mikachan