feat(developer): remove Delphi package installer compiler 🎺 #9616
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keymanapp-test-bot
bot
added
has-user-test
user-test-missing
User Test ResultsTest specification and instructions User tests are not required |
keymanapp-test-bot
bot
changed the title
keymanapp-test-bot
bot
removed
the
user-test-missing
mcdurdin
requested review from
rc-swag,
ermshiperete,
jahorton and
darcywong00
as code owners
ermshiperete
approved these changes
Sep 29, 2023
Base automatically changed from
feat/developer/kmc-package-windows-installer
to
epic/package-metadata
October 3, 2023 13:35
…er-without-buildpkg
mcdurdin
added a commit
that referenced
this pull request
Nov 14, 2023
This is a regression coming out of #9616. The new setup.inf generation process is done in a Makefile and inadvertently added significant whitespace, which was not being trimmed by setup.
rc-swag
pushed a commit
that referenced
this pull request
Nov 14, 2023
This is a regression coming out of #9616. The new setup.inf generation process is done in a Makefile and inadvertently added significant whitespace, which was not being trimmed by setup.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8340.
This removes the legacy package installer compiler code, and strips out references to it from Keyman Developer. At the same time, this required a refactor of the code in the Keyman for Windows installer build, which used the CompilePackageInstaller code to build the redistributable installer. As the steps required were actually quite simple, I've gone ahead and reimplemented them directly in the Makefile.
Removing the UI for compiling package installers may be worth discussion. I opted not to reimplement the UI using the
kmcwrapper, because the package installer is an advanced use case which typically should not be the default distribution method for any developers. It is still possible to build the installer using the command line withkmc, and that's the recommended way forward for those who still have a need to build redist installers.Given that the majority of tech support responses to issues arising with use of this feature has been to point the keyboard developer away from the feature, I am personally comfortable with this. I do plan to make a KB article on how to transition from the UI to the command-line, based on the example in developer/src/kmc/README.md:
keyman/developer/src/kmc/README.md
Lines 53 to 63 in a1a7b39
@keymanapp-test-bot skip