Prevent Dev title from showing up on Store builds#627
Merged
HowardWolosky merged 1 commit intomicrosoft:masterfrom Aug 6, 2019
Merged
Prevent Dev title from showing up on Store builds#627HowardWolosky merged 1 commit intomicrosoft:masterfrom
HowardWolosky merged 1 commit intomicrosoft:masterfrom
Conversation
Commit 0722781 updated the app to use `DevAppName` for the app's window title when it was a non-store build, based on the state of `IsStoreBuild`. Unfortunately, `IsStoreBuild` is a _project_ level variable defined in [build-app-internal.yaml](https://github.com/microsoft/calculator/blob/0722781fc60565938da42ea252766b30d02e5fb5/build/pipelines/templates/build-app-internal.yaml#L36), but not a _compile-time_ defined value. To solve this, we are now defining `IS_STORE_BUILD` in `Calculator.vcxproj` when `IsStoreBuild='True'`, the same way that we set `SEND_DIAGNOSTICS` for official builds, and we'll change the window title based on that new `#define`. Using this new `#define` can lead us down a slippery slope. We need to limit the amount of divergent code that we have between dev/official builds. This should be hopefully one of very few instances where this value is ever used.
grochocki
approved these changes
Aug 6, 2019
sanderl
approved these changes
Aug 6, 2019
Contributor
sanderl
left a comment
sanderl
left a comment
There was a problem hiding this comment.
Looks good. My adhoc tests passed. :)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Commit 0722781 updated the app to use
DevAppNamefor the app's window title when it was a non-official build, based on the state ofIsStoreBuild.Unfortunately,
IsStoreBuildis a project level variable defined in build-app-internal.yaml, but not a compile-time defined value.To solve this, we are now defining
IS_STORE_BUILDinCalculator.vcxprojwhenIsStoreBuild='True', the same way that we setSEND_DIAGNOSTICSfor official builds, and we'll change the window title based on that new#define.Using this new
#definecan lead us down a slippery slope. We need to limit the amount of divergent code that we have between dev/official builds. This should be hopefully one of very few instances where this value is ever used.Description of the changes:
#definenamedIS_STORE_BUILDwhen the project flagIsStoreBuildis set toTruefor theCalculatorproject.IsStoreBuildwhen determining the window title name to instead check againstIS_STORE_BUILDHow changes were validated:
IS_STORE_BUILDandSEND_DIAGNOSTICSwere not set. Then, modified the project file and changed the check to beIsStoreBuild != 'True'and ran the same checks again, and verified that this time bothIS_STORE_BUILDandSEND_DIAGNOSTICSwere set.