Tagging #if statements for Az #7712
Conversation
…er Compute. Updated Compute #if statements.
# Conflicts: # src/ResourceManager/Profile/Commands.Profile/Account/ConnectAzureRmAccount.cs
|
@MiYanni Please fix merge conflicts. |
| @@ -18,6 +18,7 @@ namespace Tools.Common.Helpers | |||
| { | |||
| public static class EnvironmentHelpers | |||
| { | |||
| // TODO: Remove IfDef code | |||
There was a problem hiding this comment.
What actually happens if we remove this code?
There was a problem hiding this comment.
This code is unused in Az as the way that modules are loaded has changed. AppDomains don't exist in NetStandard/Core.
There was a problem hiding this comment.
Yes, I think this is only relevant for the dependency analyzer below.
| #if !NETSTANDARD | ||
| AppDomain.Unload(_appDomain); | ||
| #endif | ||
| issueLogger.Decorator.Remove("AssemblyFileName"); |
There was a problem hiding this comment.
Az analysis does not use an AppDomain. AppDomains don't exist in NetStandard/Core.
| public HelpAnalyzer() | ||
| { | ||
| Name = "Help Analyzer"; | ||
| } | ||
| public AnalysisLogger Logger { get; set; } | ||
| public string Name { get; private set; } | ||
|
|
||
| // TODO: Remove IfDef code |
There was a problem hiding this comment.
same question - what happens when we remove the remote appdomain? Doesn't this impact assembly loading?
There was a problem hiding this comment.
Az analysis does not use an AppDomain. AppDomains don't exist in NetStandard/Core.
| EnvironmentHelpers.CreateProxy<AssemblyLoader>(directoryPath, out _testDomain); | ||
|
|
||
| // TODO: Remove IfDef | ||
| #if NETSTANDARD |
There was a problem hiding this comment.
In this case, particularly, how is execution impacted by not using a remote appdomain? This funntionality isn't available in netcore, but I'm wondering how we can continue the same detection of conflicting assemblies if we don't have some capability to load two versions of the same assembly
There was a problem hiding this comment.
@cormacpayne Have any insight for this?
|
|
||
| try | ||
| { | ||
| var assembly = Assembly.LoadFrom(assemblyPath); | ||
| foreach (var type in assembly.GetCmdletTypes()) | ||
| { | ||
| var cmdlet = type.GetAttribute<CmdletAttribute>(); | ||
| // TODO: Remove IfDef code |
There was a problem hiding this comment.
How do we get eauivalent functionality in netstandard?
There was a problem hiding this comment.
I'm not sure. This would be either @praries880 or @cormacpayne domain.
| * Update cmdlet Test-AzureRmNetworkWatcherConnectivity, add validation on destination id, address and port. | ||
| * Minor changes for upcoming AzureRM to Az transition | ||
|
|
||
| ## Version 6.10.0 |
src/ResourceManager/KeyVault/Commands.KeyVault/Models/KeyVaultManagementCmdletBase.cs
Outdated
Show resolved
Hide resolved
src/ResourceManager/KeyVault/Commands.KeyVault/Models/KeyVaultManagementCmdletBase.cs
Show resolved
Hide resolved
| @@ -0,0 +1,257 @@ | |||
| // ---------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
this compute stuff looks like it doesn't belong here - please remove or make sur ethis is chedked in separately
There was a problem hiding this comment.
This allowed me to edit the changes for these files without affecting the ServiceManagement code, which shouldn't be modified. Make sense? This work needed to be done anyway, so it was much easier to do this now while I'm updating the rest of the files for Az.
| @@ -0,0 +1,27 @@ | |||
| // ---------------------------------------------------------------------------------- | |||
There was a problem hiding this comment.
let's split out the compute sync/vhdmanagement additions into a separate pr
There was a problem hiding this comment.
It is literally copy-paste files and changing them to be tagged for Az. Does it save us time or effort to split it into a new PR?
There was a problem hiding this comment.
So, if it isn't easier to make this part of deleting the servicemanagement and Net452 artifacts, this is fine
There was a problem hiding this comment.
I need these changes for this PR: #7913
# Conflicts: # src/ResourceManager/Automation/Commands.Automation/Common/RequestSettings.cs # src/ResourceManager/Compute/Commands.Compute/ChangeLog.md # src/ResourceManager/RecoveryServices/Commands.RecoveryServices/ChangeLog.md # src/ResourceManager/Resources/Commands.ResourceManager/Cmdlets/Json/CamelCasePropertyNamesWithOverridesContractResolver.cs # src/ResourceManager/Resources/Commands.Resources/ChangeLog.md # src/ResourceManager/Sql/Commands.Sql/ChangeLog.md # src/ResourceManager/Sql/Commands.Sql/Common/AzureEndpointsCommunicator.cs # src/ResourceManager/Sql/Commands.Sql/VulnerabilityAssessment/Services/BaseSqlVulnerabilityAssessmentAdapter.cs # src/Storage/Commands.Storage/ChangeLog.md
…rshell into ifdef-tagging # Conflicts: # src/ResourceManager/Compute/Commands.Compute/ChangeLog.md # src/ResourceManager/Network/Commands.Network/ChangeLog.md # src/ResourceManager/Resources/Commands.Resources/ChangeLog.md # src/ResourceManager/Sql/Commands.Sql/ChangeLog.md # src/ResourceManager/Sql/Commands.Sql/Common/AzureEndpointsCommunicator.cs
Description
This is for: #6793
Key
#if NETSTANDARDwas simply tagged with either// TODO: Remove IfDefor// TODO: Remove IfDef code. These changes are unlikely to cause issues and/or won't require any special attention for migration.#if NETSTANDARDwas removed and not necessary.Checklist
CONTRIBUTING.mdplatyPSmodule