[MM-14740] Consolidate configuration to support integration of MSI/GPO #959
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
- config needs to load in each process (main and renderer) and then synchronize with each other - finished saving ui functionality
|
Hmm, tests run fine locally ... |
|
enahum
requested changes
Apr 10, 2019
enahum
reviewed
Apr 10, 2019
|
Regarding tests, that failing test works fine locally and I'm not entirely sure why it wouldn't work here. I'm going to try something though ... |
Can’t seem to get this test to work, even though what is being tested works fine in the actual app.
click of ‘light’ option wasn’t triggering an update as it is selected by default, so flipped the order to first select ‘dark’ and then ‘light’
|
Okay, so test is fixed. The test was initially trying to 'click' on the |
enahum
approved these changes
Apr 10, 2019
|
Ok for me. Tested on Windows and Linux. |
wget
approved these changes
Apr 11, 2019
|
Awesome!! Lets add the MSI stuff |
hanzei
added
4: Reviews Complete
JtheBAB
pushed a commit
to JtheBAB/desktop
that referenced
this pull request
Jan 17, 2020
mattermost#959) * config logic consolidation * filter out duplicate servers * build default teams and GPO teams are not editable * tweaks * tweak config architecture to support tests - config needs to load in each process (main and renderer) and then synchronize with each other - finished saving ui functionality * add esdoc comments to new config module * remove old config-related files * revert eslint comment * don’t filter teams, duplicates are allowed * some code review tweaks * Remove unecessary deepCopy * tweak for tests * Skip test for now Can’t seem to get this test to work, even though what is being tested works fine in the actual app. * fix for failing test click of ‘light’ option wasn’t triggering an update as it is selected by default, so flipped the order to first select ‘dark’ and then ‘light’
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.
Before submitting, please confirm you've
npm run lint:jsfor proper code formattingPlease provide the following information:
Summary
This PR consolidates configuration handling (loading, combining default/build configs, updating, saving) in preparation for integrating Windows GPO data (function stub included). The new configuration object is instantiated in the main thread and any primary views loaded in the renderer thread. Each instance is setup with the ability to notify the other instance of changes in order to maintain synchronization between loaded config data in the main and renderer processes. This multi instance setup to needed to support existing tests and their access to config data.