-
-
Notifications
You must be signed in to change notification settings - Fork 112
Principles of Keyman Code Changes
These are some general principles that we need to be following when we create features or bug fixes in Keyman. These principles help to uphold a good user experience for Keyman end users and keyboard/model developers.
We haven't always been consistent in following these principles and that's something I'd like to improve on.
[Legacy: We tracked the initial discussion in this google doc.]
For example, we don't make changes to the keyboard key caps, nor do we insert an extra key into a popup menu. We process the rules according to the documented specification and we don't deviate from that.
2. Engine backward compatibility: A compiled keyboard or model, compiled in an earlier version MUST work unmodified in a later version.
Remember that we don't control all keyboards and models. There are many, many keyboards not in our repository, and there will also be models that are not in our repository.
Generally, I won't approve breaking changes in interfaces for models and keyboards: a 13.0 model MUST continue to work with 14.0, 15.0 and into the future. Keyman 6.0 keyboards from the 90's work with Keyman 14.0. In theory, Keyman 4.0 keyboards from 1996 would work even though I don't know if any still exist (and wouldn't be Unicode so would definitely be 'legacy').
One reason for this is that keyboards and models are distributed to users, and users may not be able to acquire a new version of the package from the package developer, nor downgrade to an earlier version of Keyman to continue using their package. Thus, an upgrade of Keyman must not break existing packages.
We cannot assume an evergreen software world for Keyman, where everything is always updated.
3. Compiler backward compatibility: A new version of the compiler MUST be able to compile a keyboard, model or package that targets an earlier version of Keyman.
The Keyman keyboard language store(&version)
version targeting statement is designed to help backward compatibility. It allows developers to target a specific version of the Keyman Engine and avoid using new functionality that won't work on that version. It directs the compiler to produce a keyboard/package that will work in that earlier version.
4. Source backward compatibility: Keyboard, model and package source file formats SHOULD be backward compatible.
That is, changes to the compiler SHOULD not require keyboard or model developers to make changes to existing keyboards simply in order for them to compile with a new version.
It's okay to add compiler deprecation warnings for code that is no longer recommended. These warnings SHOULD not be shown where a keyboard/model has an older version target.
It may be okay, well down the line, to have a compiler error, for example where newer replacement code works better. This error SHOULD not be shown where a keyboard/model includes an older version target statement.
An example of this is language tagging. This is now done in packages, rather than in keyboards. However, the existing language tag statements (store(ðnologuecode)
for example) continue to work.
Casing rules, normalization, and other similar one-size-fits-all patterns are helpful but where we implement them, we MUST provide a pathway for a keyboard/model developer to override them.
An API consumer that was built for Keyman 13 should continue working with Keyman 14. This is particularly applicable for desktop APIs where the version of Keyman is determined by the end user rather than the API consumer. On mobile devices, the APIs are used only by a Keyman Engine library consumer, and so there is a pathway for deprecations and changes.
Changes to APIs SHOULD be marked with deprecations for at least one major version.
An exception to this principle may be made for security requirements.
Now you can probably see some potential for conflict between these principles. We'll fix bugs and that includes some design bugs. But even when fixing bugs we need to make sure that we don't violate the backward compatibility principles.
These may be illustrated with real case studies.
U+25CC (#3039)
We started adding a dotted circle to unattached diacritics to try and fix an issue on Android where unattached diacritics would display off-key. This however violates Principle 1 (and 5).
In Keyman 14, we are winding this back, which in theory is a backward compatibility violation (principle 3). The keyboard may not display as intended on older versions of Keyman, where a keyboard designer has assumed the presence of the U+25CC. However, this conflict can be resolved by the keyboard designer if it is important, by including a custom OSK font which guarantees the display of the characters in the right way regardless of the version.
Showing the base key in the popup menu (#3718)
This design decision violates Principle 1. Somewhat less obviously, this ended up breaking Principle 5 too.
It came into being way back when we first developed Keyman for iPhone. The idea was that we would match the default pattern of showing the base key in the popup menu that the default device keyboard did on some devices, but did not do on other devices, e.g. iPads.
Principle 5: the order of letters in the popup menu matters, for example for Amharic, where the base letter should be shown but not in first place!
So we'll remove this functionality in Keyman 14; this slight change in functionality does not break keyboards, although there will be a slight change in user experience. But in doing so, we give control back to the keyboard developer.
So following Principle 1 helps us to uphold Principle 5.
Centralising word breaking functionality (#3706)
Here, we realised we didn't need to implement the full word breaking algorithm in every model, but could centralise the code. However, in so doing, we would end up breaking custom models.
This is a design improvement. But it violates principles 2, 3 AND 4:
2: existing custom models 'compiled' with Keyman 13 would not work in Keyman 14. 3: a custom model compiled with Keyman 14 may not work in the same way in Keyman 13. Would trie models compiled with Keyman 14 work with Keyman 13? This is unclear to me. 4: custom model designers would need to make changes to their model in order to conform to the new interface, just to get it to compile with Keyman 14.
Keyman 14 will allow us to support keyboards with und-fonipa
BCP 47 tag. However, Keyman 13 on Windows did not allow us to do this. How do we support this transition? This is an open question for me.
Copied from https://github.com/keymanapp/keyman/pull/6786#issuecomment-1159923816
We are trying to raise the bar on alpha PRs -- we want to be confident that they are solid when merged. So if you are not 100% confident that this is not going to break things, then we need to be doing testing before merge to catch the issues. That's the job of the test team, and unit tests.
I'll try and articulate, hopefully somewhat clearly.
During release retrospective, we identified that we had a lot of issues in the beta cycle that we should really have picked up earlier in the release cycle. This led me to conclude:
- Time-separation between when we make the changes and when we thoroughly test (during beta) makes it much harder to trace which PR is the root cause.
- Fixes take longer, because we've all context-switched in the mean time.
- If original PR author is not available during beta, then it's much harder to fix!
- We end up with more noise to triage - more crash reports, more bug reports.
All of these hamper us in our chase for a shorter release cycle. So, as the team has grown (yes, we are in a pinch this week ... but not generally), and we have more capacity to raise the quality bar, I think we can afford to slow down a little on our PRs and make sure they are really good.
This is all subjective to a degree of course. For example, I won't merge the 🎢 PR chain until all the end-user-impacting issues are tested and sorted. But I may merge it before 100% of the code layout impurities are addressed -- because this is a net improvement without additional regressions.
In terms of prioritisation in PRs, for end-user facing UX:
- Build breaking should be prioritised over everything else (it blocks the entire team).
- Regressions should always be fixed before merge to 'release branches' (
master
/beta
/stable-x.y
): we never want a PR that regresses existing behaviour. - New features should be complete before merge to release branches: we develop them in a PR chain if they are big. AKA 'feature branches'. We can even protect the topmost branch in the feature branch in GitHub to help remind us that child PRs should be clean before merge.
- Fixes for regressions identified post-merge should be prioritised over new development or new bug fixes (somewhat subject to additional; triage).
- Long-lived branches should be rebased or have their base merged in prior to final testing, to ensure we haven't introduced any conflicts.
Now that we have solid test and deploy environments for all platforms in all PR builds, I think this is achievable. We no longer need to merge to alpha in order to actually use Keyman.
As always, these priorities are subject to triage and negotiation -- these are guidelines, but they are solid ones that we will follow as far as we can. Flexibility still needs to stay a part of our process!
And yeah, I don't think any of this is rocket science, but more a reflection of the fact that as the Keyman codebase is maturing and the team is growing, we will continue to identify places where we can improve our processes, and it's exciting to be able to do so.
Finally, if you see places where my code changes don't meet this bar, please do let me know (kindly!). I'm sure I'm not always doing a great job of keeping up with this (or even articulating it clearly), particularly in my current situation.