-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
Naijatype experimental: flicks, v17 & documentation updates #2822
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. |
There seems to be a conflict in the description in the .kps that needs resolving:
I think you can use the "web editor" that is linked to above to resolve that. |
If there isn't going to be major redesign of the keyboard, renaming the foldername/filenames, etc, and if you have adequate documentation, we are happy for you to move it to |
Can you open this as a new feature issue at https://github.com/keymanapp/keyman/issues/new/choose? |
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. |
The whole build system was recently updated (and every keyboard slightly modified to follow Keyman 17 conventions). But that happened last week sometime, so I think the fact it is showing yesterdays date might be a regression in what is displayed. I looked at another keyboard and see the same thing. @mcdurdin was this intentional? |
The keyboard won't build. Here are the error messages:
I believe you already converted the project to Keyman 17 format. But, I'm not quite sure about the minimum Keyman version needed. Try updating this statement |
To try to help it build in the repository
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. |
Interesting. I've bumped the minimum Keyman version required (though perhaps that leaves some users behind?) and have done what I can to remove the warnings. I'm hoping the flick gestures will just be ignored on Keyman < 17, and I don't need to make this Keyman 17 only.
|
<CheckFilenameConventions>False</CheckFilenameConventions> | ||
<ProjectType>keyboard</ProjectType> | ||
<Version>2.0</Version> | ||
<BuildPath>$PROJECTPATH\..\LexicalModels\build</BuildPath> |
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.
This line is what caused the build failure -- the .kmx is being put into the wrong folder. This line should be removed (default is $PROJECTPATH/build
, which is what we want):
<BuildPath>$PROJECTPATH\..\LexicalModels\build</BuildPath> |
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.
Thank you. Sorry I missed that.
You shouldn't need to specify the version number; if you leave the |
This is a side-effect of every release and experimental keyboard in the repository being touched in order to move the .keyboard_info metadata into the .kps file. Avoiding reporting of this change would require a non-trivial change to the build process, as currently the build uses the data in the git repository to determine the last date of modification (code). I opted not to try because I didn't think it was important enough, but we could give it a go if other people think it is important. |
@mcdurdin pointed out it‘s not needed any more.
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. |
this broke the build
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. |
What struck me as odd though was that the version number reported (56) was itself last modified some weeks ago, so the 'last modified' and 'latest keyboard version number' were not connected in any way. But I accept this is only likely to show up in the rare circumstance that an update has been pushed and a PR is active but not yet accepted. Would it be possible to get the last-modified from the same revision that the 'Latest Version' comes from? Not worth it if there's lots of work to do that. |
Yes, because we didn't update version numbers, so this only impacts the metadata shown on the website. However, this discussion prompted keymanapp/keyman#11793, which should be straightforward to implement, and would mitigate the problem, so 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. |
Included more languages that Naija Ty[e definitely covers, a superset of the SIL Nigeria 3 legacy keyboards.
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. |
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 can't find the review I did yesterday. I must have closed it without saving. Please open your kpj project in developer and compile your .kmn file. If you can address some of those warning it might be useful. Then, go to the .kps and try to compile. You will see these messages:
You've either renamed some of your files or deleted them. Also, the font name is not correct. Either put Andika-Regular.ttf in your commit or change the .kps to reference AndikaAfr-R.ttf. The errors must be resolved. The warning are related to not having a minimum Keyman version in your header in the .kmn so I guess that's okay. |
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. |
Thanks. I switched to Andika (full) and seem to have left behind the non-text updates when I staged from my development folder to the local repo. Thanks for noticing that. |
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. |
…eyboards into naijatype-experimental
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. |
OK, I now have a new regime of building in my local repo before I push. Should I be worried if I push and then see 'TeamCity build failed'? Sorry for taking up so much of your time @LornaSIL |
I know it's confusing. No, you don't need to be concerned that it says "Some checks were not successful". We don't automatically kick off a build until we've done preliminary checking so that our build agent doesn't have to work too hard. When there are a lot of changes to a keyboard I'll generally do a build on my system first to make sure it builds for me before I ask our agent to do a build. In this case, it builds for me, so I'll go ahead and see if our system is also happy with it :) |
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
I've added a lot of Keyman 17 features taking advantage of flicks and tried to update and improve the documentation. At some point I'll be interested in any advice to take this from 'experimental' to the main repository, if that is helpful. Feedback very much useful. One niggle I have is that I can't see how to display different languages not just the same keyboard name in the menu:
![IMG_0166](https://private-user-images.githubusercontent.com/420492/339401734-3b967f1e-31d2-4bd4-99ce-9dd63b68a2ba.jpg?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjEyNjUzNTEsIm5iZiI6MTcyMTI2NTA1MSwicGF0aCI6Ii80MjA0OTIvMzM5NDAxNzM0LTNiOTY3ZjFlLTMxZDItNGJkNC05OWNlLTlkZDYzYjY4YTJiYS5qcGc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwNzE4JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDcxOFQwMTEwNTFaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT04M2RlNDU0Y2E3OTQyMzU0MDg2NmI0NGVjMGNhOGU2M2Y3ZmIzYTEzZDVlNmQ4NDM2ZDZmZjViMmU3M2Y3NzI0JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.kkbuGAHbrmL9TAsH-iSCxqvodVACdMEcC6T_jvyECH0)