-
Notifications
You must be signed in to change notification settings - Fork 357
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
User/ejohn/settings #121
User/ejohn/settings #121
Conversation
@@ -1,7 +1,7 @@ | |||
// Copyright (c) Microsoft Corporation and Contributors | |||
// Licensed under the MIT license. | |||
|
|||
namespace DevHome.Core.Contracts.Services; | |||
namespace DevHome.Common.Contracts; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we come up with something other than "Common"? That could be a grab bag of a whole bunch of stuff and is ambiguous. Is this common to all plugins? Common to DevHome? Common to Windows? Also why is common not under src? Is it not src? There's common\helpers and src\helpers, common\services and src services - what's the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dev Home is split between core app and tools. "Common" is shared between the two. "src" is just the core application source code. Ideally, the "common" code would live in "src", but then we have a circular dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be fine with actually getting rid of the "common" namespace and just making all namespaces in DevHome.Common.* be DevHome.*, but that would definitely be a separate change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dev Home is split between core app and tools. "Common" is shared between the two. "src" is just the core application source code. Ideally, the "common" code would live in "src", but then we have a circular dependency.
I would expect all source for shipping code to be under source, and if it's just the core application then that should be refactored into a sub folder of src for the app. Or change 'src' to be indicative that it is the app and not have a src folder.
Looks good to me. There will be some conflicts with #120. Let me know if you need help. |
…vhome into user/ejohn/settings
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Summary of the pull request
Add in depth Settings page.
References and relevant issues
Detailed description of the pull request / Additional comments
Here's is the detailed list of what is currently available in this Settings page change.
Top Level Page: Has 4 blades - Preferences, Accounts, Plugins, About
Preferences: Theme settings (light/dark/default)
Accounts: Login button and list of currently signed in accounts.
Plugins: List of all installed plugins as blades. Each blade has a toggle switch to enable/disable the plugin.
About: About info for Dev Home.
This to add in future:
Validation steps performed
PR checklist