change(developer): use full github url in kmc copy parameters #12754
Conversation
User Test ResultsTest specification and instructions User tests are not required |
| ref.path = '/' | ||
| }; |
There was a problem hiding this comment.
| ref.path = '/' | |
| }; | |
| ref.path = '/'; | |
| } |
| const parts = source.split(':'); | ||
| const id = parts[1]; | ||
| const parts = CLOUD_URI.exec(source) ?? KEYMANCOM_CLOUD_URI.exec(source); | ||
| const id: string = parts.groups.id; |
There was a problem hiding this comment.
| const id: string = parts.groups.id; | |
| if(!parts) { | |
| throw new Error('Expected CLOUD_URI or KEYMANCOM_CLOUD_URI to match'); | |
| } | |
| const id: string = parts.groups.id; |
| /** @internal */ | ||
| public unitTestEndPoints = { | ||
| getGithubSourceProject: this.getGitHubSourceProject.bind(this), | ||
| getCloudSourceProject: this.getCloudSourceProject.bind(this) | ||
| }; |
There was a problem hiding this comment.
This pattern allows us to expose private functions for unit testing without breaking encapsulation. We use a similar pattern for global functions in a module, where we don't want to export them directly.
@markcsinclair fyi.
https://github.com/keymanapp/keyman/wiki/Unit-Tests#typescript
mcdurdin
requested review from
darcywong00,
SabineSIL,
markcsinclair and
srl295
as code owners
| const KEYMANCOM_CLOUD_URI = /^(?:http(?:s)?:\/\/)?keyman\.com\/keyboards\/(?<id>[a-z0-9_.-]+)/i; | ||
| const CLOUD_URI = /^cloud:(?<id>.+)$/i; | ||
|
|
There was a problem hiding this comment.
It's only a few lines, but since GitHubUrls got refactored, thoughts on pulling these into a separate cloud-uri.ts file? (along with some comments about the regexes)
| const parts = source.split(':'); | ||
| const id = parts[1]; | ||
| const parts = CLOUD_URI.exec(source) ?? KEYMANCOM_CLOUD_URI.exec(source); | ||
| const id: string = parts.groups.id; | ||
|
|
||
| const isModel = /^[^.]+\.[^.]+\.[^.]+$/.test(id); |
There was a problem hiding this comment.
Would one of the regexes work?
There was a problem hiding this comment.
no, because those regexes all have .model. in them. We could build a shared regex I guess but I am not really keen on more refactoring for it.
| github:keymanapp/keyboards:release/k/khmer_angkor/khmer_angkor.kpj | ||
| github:keymanapp/keyboards:/release/k/khmer_angkor/khmer_angkor.kpj` | ||
| ); | ||
| // 0x0011 unused |
There was a problem hiding this comment.
Noting this PR removes ERROR_InvalidGitHubSource.
Will https://help.keyman.com/developer/18.0/reference/messages/km0b011 automatically disappear in the sync?
There was a problem hiding this comment.
It should do, we'll see when we deploy?
|
NOTE: I will need to cherry-pick this to |
…es-from-remotes-2' into change/developer/12746-kmc-copy-full-gh-url
… into change/developer/12746-kmc-copy-full-gh-url
Fixes: #12746
@keymanapp-test-bot skip