-
-
Notifications
You must be signed in to change notification settings - Fork 270
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
GFF Sheek Bakrii Saphaloo v1.0.7 #2736
Conversation
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Thanks for the PR! We will resume checking keyboards after the migration to Keyman 17 happens. |
This is a keyboard repo and as such I really don't need to approve font naming, but I do want to make you aware that font names should really not exceed 31 characters. The issue with this font name (not filename) is that internally it will also have In font naming these days the foundry is kind of frowned upon included in the fontname. In this case "Athinkra " could potentially be removed from the name and it would make the length technically more widely acceptable. I know SIL has a lot of "SIL" in font names, but we don't include it in any of our newer fonts. Since Athinkra is presumably listed in the license and copyright fields it will definitely be acknowledged that Athinkra created it. Anyways, something to consider and maybe you have no control over the fontname since it's a separate org. Here is a page on font naming: https://silnrsi.github.io/FDBP/en-US/Font_Naming.html but it doesn't go into the length of the name. FontBakery has this issue that describes some of the pitfalls of long names: fonttools/fontbakery#2179 They start out with saying 20 characters is the top length, but the discussion seems to end at 31. |
Thanks @LornaSIL for the discussion and references, I will review them. I found the name length limit by trial-and-error with Microsoft Word font menus. "Athinkra: Sheek Bakrii Saphaloo" (31 chars) was the longest that it would take before chopping off the last character (I had wanted to use "Athinkra - Sheek..." but this added one more character. However, I later found that LibreOffice Writer did not like the colon in the name, which hadn't been an issue elsewhere (maybe a bug with LibreOffice, I haven't found that colon is illegal, but current naming rules have been hard to find). Removing the ":" goes to 30 chars. Adding Other than the font name change, the PR does contain one keyboard input update also. |
Do you plan to change the font name? If you are, it would be much better to do it now rather than later. I think if font names keep switching people may end up with multiple versions of the font on their system. I'd rather get it right now than wait. |
Thanks @LornaSIL , I understand the naming issue better now and am taking it up with the Athinkra group. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Hi @LornaSIL , I've done a number of name changes and think this is ready for review again. Thanks! |
Hi. I'm traveling, but will be back online Monday. In the meantime, can you
merge master into your PR? In fact, your whole repo could be updated as
every keyboard had changes since we moved to Keyman 17 format. This means,
the .kpj file is fairly empty. The keyboard_info file needs deleting,
everything that was in keyboard_info was moved to .kps. So, I expect you'll
have merge conflicts. I hope you can resolve them.
…On Fri, Jun 7, 2024, 5:35 AM Daniel Yacob ***@***.***> wrote:
Hi @LornaSIL <https://github.com/LornaSIL> , I've done a number of name
changes and think this is ready for review again. Thanks!
—
Reply to this email directly, view it on GitHub
<#2736 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABV6RI64CTPKRJUCT4LKHXDZGGEIHAVCNFSM6AAAAABHIJQVICVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJUGU3DKMRUGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
1 similar comment
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
I think this keyman project has now been successfully ported to v17 and synched with the head branch. Thank You. |
Thank you for your pull request. The Keyman keyboard review team have been notified of your pull request and will review it and build it shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
::
added for typing the fullstop (supports a user tendency carried over from Ethiopic typing).