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

application-package: refine config typings #9568

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

paul-marechal
Copy link
Member

Rework how the different application configurations are defined to allow
partial setting when calling configuration providers.

Use default values when setting a partial configuration through the
configuration providers.

How to test

Setting partial objects when calling FrontendApplicationConfigProvider.set and BackendApplicationConfigProvider.set should work and properly use defaults.

Review checklist

Reminder for reviewers

@paul-marechal paul-marechal force-pushed the mp/application-props-typing branch 2 times, most recently from f7919d8 to 2e51dea Compare June 9, 2021 04:08
@vince-fugnitto vince-fugnitto added extensibility issues to simplify ability to extend Theia quality issues related to code and application quality labels Jun 9, 2021
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that:

  • partial updates with FrontendApplicationConfigProvider.set are supported.
  • partial updates with BackendApplicationConfigProvider,set are supported.
  • confirmed that there are no regressions when starting the application, the applicationName (which is updated for both example-browser and example-electron is properly updated and reflected.
  • CI is green and the new tests successfully pass.

@paul-marechal paul-marechal force-pushed the mp/application-props-typing branch from 2e51dea to 4ba5895 Compare June 9, 2021 14:56
Rework how the different application configurations are defined to allow
partial setting when calling configuration providers.

Use default values when setting a partial configuration through the
configuration providers.
@paul-marechal paul-marechal force-pushed the mp/application-props-typing branch from 4ba5895 to bcb8fbb Compare June 9, 2021 14:58
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍
The feedback I proposed for the *.spec.ts (unit-tests) have also been resolved.

@paul-marechal paul-marechal merged commit aa4fe4a into master Jun 9, 2021
@paul-marechal paul-marechal deleted the mp/application-props-typing branch June 9, 2021 16:27
@github-actions github-actions bot added this to the 1.15.0 milestone Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extensibility issues to simplify ability to extend Theia quality issues related to code and application quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants