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

Notify XAML of renames caused by Naming Rule fixes #18693

Merged
merged 3 commits into from
May 8, 2017

Conversation

dpoeschl
Copy link
Contributor

@dpoeschl dpoeschl commented Apr 13, 2017

Fixes #16562

Customer scenario: The customer renames a XAML type or element on the C# side to comply with a Naming Rule, and the name doesn't get updated on the XAML side.

Bugs this fixes: #16562

Workarounds, if any: The customer can manually update XAML references to the symbol.

Risk: Low. No other features depend on this aspect of Naming Rules, and we're already calling these VS APIs in other features. This change just makes those API calls available to Code Fixes.

Performance impact: Renaming an identifier via a Naming Rule fix will be slightly slower, but this brings it in line with the behavior of Inline Rename and Rename Tracking.

Is this a regression from a previous update?: No

Root cause analysis: This was a known missing aspect of the Naming Rules fixer that we didn't implement in RTM.

How did we miss it? This was a known missing aspect of the Naming Rules fixer that we didn't implement in RTM.

How was the bug found? Reported by a customer at https://developercommunity.visualstudio.com/content/problem/9702/naming-rule-violation-not-complete.html

@dpoeschl dpoeschl added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Apr 13, 2017
@dpoeschl dpoeschl force-pushed the NamingStylesUpdateXAML branch from a6d77ae to 7b2ecf1 Compare April 17, 2017 16:52
@dpoeschl dpoeschl removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Apr 17, 2017
@dpoeschl
Copy link
Contributor Author

Tagging @dotnet/roslyn-ide for review.

Copy link
Contributor

@rchande rchande left a comment

Choose a reason for hiding this comment

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

Tests? If this requires further integration test improvements in order to be tested, can you file a bug for followup?

@dpoeschl
Copy link
Contributor Author

@rchande I added a unit test to prove the new Operation is applied only during application and not during preview, and that it receives the correct arguments.

@rchande
Copy link
Contributor

rchande commented Apr 19, 2017

Thanks @dpoeschl! Looks good.

@dpoeschl
Copy link
Contributor Author

dpoeschl commented Apr 19, 2017

retest ubuntu_14_debug_prtest please
failure: https://ci.dot.net/job/dotnet_roslyn/job/master/job/ubuntu_14_debug_prtest/1966/

@dpoeschl
Copy link
Contributor Author

Tagging @MattGertz for Ask Mode consideration. The test failures are unrelated, but I'm working on getting them green.

@MattGertz
Copy link
Contributor

Approved -- as noted in side mail, you're also working on getting the tests fixed.

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 1, 2017

retest windows_debug_vs-integration_prtest please

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 1, 2017

retest windows_release_vs-integration_prtest please

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 1, 2017

"xunit produced no error output but had exit code -2146233082" -- @dotnet/roslyn-infrastructure This happened on both of the failing runs. Is something generally wrong here? ExecutionEngineException...

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 2, 2017

retest windows_debug_vs-integration_prtest please

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 2, 2017

retest windows_release_vs-integration_prtest please

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 2, 2017

retest windows_debug_vs-integration_prtest please

https://ci.dot.net/job/dotnet_roslyn/job/master/job/windows_debug_vs-integration_prtest/3606/

Roslyn.VisualStudio.IntegrationTests.CSharp.CSharpWinForms.ChangeControlProperty [FAIL]

Filed as #19191

@dpoeschl
Copy link
Contributor Author

dpoeschl commented May 4, 2017

retest windows_debug_vs-integration_prtest please

@dpoeschl dpoeschl merged commit 2a6c6d0 into dotnet:master May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The Naming Styles CodeFix does not update XAML references
5 participants