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.

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

feat(developer): eliminate source .model_info and .keyboard_info files 🎺 #9535

Merged

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Sep 1, 2023

Removes all references to source .keyboard_info and .model_info files in the kmc compiler and types. Updates the model info compiler to support building purely from model sources.

Adds support for calculating isRTL and license fields in .model_info compiler.

Also:

  • establishes @keymanapp/developer-utils shared module
  • moves license validation into @keymanapp/developer-utils
  • refactors kmc-model-info to a class and general cleanup.

@keymanapp-test-bot skip

Removes all references to source .keyboard_info and .model_info files in
the kmc compiler and types. Updates the model info compiler to support
building purely from model sources (isRTL, license fields are TODO).
Adds support for calculating isRTL and license fields in .model_info
compiler.

Also:
* establishes @keymanapp/developer-utils shared module
* moves license validation into @keymanapp/developer-utils
* refactors kmc-model-info to a class and general cleanup
@keymanapp-test-bot keymanapp-test-bot bot changed the title feat(developer): eliminate source .model_info and .keyboard_info files feat(developer): eliminate source .model_info and .keyboard_info files 🎺 Sep 1, 2023
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S20 milestone Sep 1, 2023
@github-actions github-actions bot added the feat label Sep 1, 2023
Comment on lines +69 to +71
if(activity.sourceExtension == KeymanFileTypes.Source.Project) {
return await this.buildTarget(this.project.projectFile, activity);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

.keyboard_info and .model_info are built from the project

@mcdurdin mcdurdin marked this pull request as ready for review September 1, 2023 04:35
@mcdurdin mcdurdin modified the milestones: A17S20, A17S21 Sep 1, 2023
Works around npm/cli#3466 when bundling internal dependencies using the
bundleDependencies package.json property.

This change works in tandem with the npm pack/publish process -- when we
run `developer/src/kmc/build.sh publish` (or `pack`), we end up with
`npm version` stomping on all our package.json files, so the repo is
dirty after this. We need a copy of the top-level package.json before
this stomping happens, in order to get a simple map of the location of
each of our internal dependencies, from the `dependencies` property (it
would be possible to figure this out with a lot more parsing of
our package.json files, but this is simpler).

This means, in future, we should avoid publishing our internal
dependencies such as those under common/ to npm, as they serve no
practical purpose there.
<Filepath>dmg.dv.test.model_info</Filepath>
<FileVersion></FileVersion>
<FileType>.model_info</FileType>
</File>
Copy link
Contributor

@darcywong00 darcywong00 Sep 11, 2023

Choose a reason for hiding this comment

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

Should we also clean up .keyboard_info files in other web test folders?

web/src/test/manual/web/issue917-context-and-notany/test_917/test_917.keyboard_info
web/src/test/manual/web/issue2924/source/testvariable/testvariable.keyboard_info
web/src/test/manual/web/issue3701/test3701/test3701.keyboard_info

Copy link
Member Author

Choose a reason for hiding this comment

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

See #9551

model_info.packageIncludes = sources.kmpJsonData.files.filter((e) => !!e.name.match(/.[ot]tf$/i)).length ? ['fonts'] : [];
model_info.version = sources.kmpJsonData.info.version.description;
model_info.minKeymanVersion = minKeymanVersion;
model_info.helpLink = HelpRoot + model_info.id;
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to be publishing lexical-model help? I didn't see HelpRoot in the help.keyman.com repo
https://help.keyman.com/model

Copy link
Member Author

Choose a reason for hiding this comment

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

This leaves the option open for the future, but no current plans 😁

Copy link
Contributor

@jahorton jahorton left a comment

Choose a reason for hiding this comment

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

Partly LGTM, partly RSLGTM.

Base automatically changed from feat/developer/use-last-commit-date-for-keyboard_info to epic/package-metadata September 28, 2023 08:54
@mcdurdin mcdurdin merged commit 65d95d3 into epic/package-metadata Sep 28, 2023
2 checks passed
@mcdurdin mcdurdin deleted the feat/developer/eliminate-source-info-files branch September 28, 2023 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants