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

Customization craftsmanship review updates #2582

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Conversation

nieubank
Copy link
Contributor

@nieubank nieubank commented Apr 10, 2024

Summary of the pull request

Misc. updates based on craftsmanship review

References and relevant issues

#2581

Detailed description of the pull request / Additional comments

  • "Developer File Explorer" should be renamed "File Explorer"
  • Margins on main page need adjustment
  • QuietBackgroundProcesses expander should default to not expanded (also removed "beta" feature placeholder text)

Additionally, renamed some code behind files to make grouping work correctly in VS.

Validation steps performed

image

PR checklist

@nieubank nieubank added Type-Craftsmanship Pertains to craftsmanship review feedback Area-Customization labels Apr 10, 2024
@krschau krschau added the Needs-Second Pull request that needs another approval label Apr 11, 2024
@@ -271,10 +271,6 @@
<value>This feature can be activated for 2 hours.</value>
<comment>A description of the Quiet Background Processes time window</comment>
</data>
<data name="QuietBackgroundProcessesExplanation.Header" xml:space="preserve">
Copy link
Contributor

Choose a reason for hiding this comment

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

Since Main/Release/Staging branches all use the same resource files, we might want not want to remove these until we confirm it's not being used in the release branch.
@EricJohnson327

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not in release yet, but staging has these strings.

I'm not we'd want to ship with this string either way.

@krschau krschau removed the Needs-Second Pull request that needs another approval label Apr 12, 2024
@nieubank nieubank merged commit 073ee2d into main Apr 12, 2024
4 checks passed
@nieubank nieubank deleted the user/nieubank/custcraft branch April 12, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Customization Type-Craftsmanship Pertains to craftsmanship review feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Customization craftsmanship review feedback
4 participants