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

Feature/GraphingCalculator initial commit #450

Conversation

danbelcher-MSFT
Copy link

@danbelcher-MSFT danbelcher-MSFT commented Apr 11, 2019

Initial PR for the feature/GraphingCalculator feature branch, part of #338.

The feature incorporates a proprietary Microsoft-owned graphing engine to drive graphing experiences in the Windows Calculator app. Due to the private nature of the graphing engine, the source available in the public repo will make use of a mock graphing engine. See README.md for more details.

This PR simply serves as a base for future feature development. As such, the PR will be immediately merged. Feedback on the content of this PR, and on the feature in general, is encouraged. If there is feedback related to the content of this specific PR, please leave comments on the PR page. We will address the comments in future PRs to the feature branch.

@danbelcher-MSFT danbelcher-MSFT merged commit 091732a into microsoft:feature/GraphingCalculator Apr 11, 2019
@danbelcher-MSFT danbelcher-MSFT deleted the feature/GraphingCalculator branch April 11, 2019 01:15
<Package xmlns="http://schemas.microsoft.com/appx/manifest/foundation/windows10" xmlns:mp="http://schemas.microsoft.com/appx/2014/phone/manifest" xmlns:uap="http://schemas.microsoft.com/appx/manifest/uap/windows10" xmlns:uap5="http://schemas.microsoft.com/appx/manifest/uap/windows10/5" IgnorableNamespaces="uap uap5 mp">
<Identity Name="Microsoft.WindowsCalculator.Dev" Publisher="CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US" Version="0.0.0.0" />
<Identity Name="Microsoft.WindowsCalculator.Graphing" Publisher="CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US" Version="0.0.0.0" />
Copy link
Member

Choose a reason for hiding this comment

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

I think this may fail to build/test in pipelines. Revert?

Same feedback for the change to AppName.

Copy link
Author

Choose a reason for hiding this comment

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

Depends. Should the branch be always ready to be merged? I didn't figure we would run CI builds for the feature branch, although it sounds like a good idea.

// C++/WinRT
#include "winrtHeaders.h"

#include "Control/Grapher.h"
Copy link
Member

Choose a reason for hiding this comment

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

nit: no project headers in the pch

Copy link
Author

Choose a reason for hiding this comment

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

I can't find an explanation for the technical reason, but there needs to be a project header in the pch in-order for the XAML compiler to generate correct type information. Removing the project header results in:

2>s:\repos\calculator\src\graphcontrol\generated files\xamltypeinfo.g.cpp(164): error C2039: 'GraphControl': is not a member of '`global namespace''

If you look in the Calculator project, the pch contains App.xaml.h. If you make a blank Visual C++ app, the pch contains 'App.xaml.h'. If you look at our other apps, it's the same. I can't find any documentation explaining why.. @jlaanstra, do you know?

@joseartrivera joseartrivera added the graphing calculator Work items related to the graphing calculator feature. label Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphing calculator Work items related to the graphing calculator feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants