Narrow down WindowsAzure.Storage usage #5479

MiYanni · 2018-02-08T18:37:18Z

Description

This is for #3430. It is an initial sweep of the projects to clean up dependencies. It removed several unused references to WindowsAzure.Storage and also allowed RedisCache to no longer require WindowsAzure.Storage (by updating a reference). The RedisCache team should review their project's changes to make sure they are good.

I did a fairly thorough check for each project to make sure that the .csproj references, the package.config, and the .psd1 aligned properly, and made sense keeping Common.Dependencies.targets in mind for the references. But, I definitely could have missed something, so a thorough review is required.

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

MiYanni · 2018-02-08T20:26:10Z

@azuresdkci Retest this please

cormacpayne

@MiYanni some comments to take a look at

cormacpayne · 2018-02-12T21:57:43Z

setup/azurecmdfiles.wxi

@MiYanni it looks like the wxi was regenerated before the CleanupBuild.ps1 script was run and removed all of the extra assemblies

@cormacpayne Are you referring to the last bullet in the guide? I didn't look at the changes to the Wxi file, because I wouldn't know what I'm looking for to be valid/invalid. I don't know Wxi files (not really something I'd assume most people are familiar with). It would probably make sense to require a clean repo prior to running the process instead of making a conditional check.

cormacpayne · 2018-02-12T22:01:43Z

...ent/Commands.ApiManagement.ServiceManagement/Commands.ApiManagement.ServiceManagement.csproj

@MiYanni it looks like there is still a reference to this package in the Commands.ApiManagement.ServiceManagement/packages.config file that should be removed

cormacpayne · 2018-02-12T22:12:06Z

src/ResourceManager/DataFactories/Commands.DataFactoryV2.Test/packages.config

@MiYanni should this be removed since it was removed in the .csproj file?

@cormacpayne If the csproj doesn't need it, why would the package.config need it?

cormacpayne · 2018-02-12T22:13:01Z

src/ResourceManager/HDInsight/Commands.HDInsight.Test/Commands.HDInsight.Test.csproj

@MiYanni it looks like this wasn't removed in the corresponding packages.config file

cormacpayne · 2018-02-12T22:14:07Z

src/ResourceManager/Profile/Commands.Profile.Test/Commands.Profile.Test.csproj

@MiYanni it looks like this wasn't removed in the corresponding packages.config file

cormacpayne · 2018-02-12T22:20:40Z

src/ResourceManager/DataFactories/Commands.DataFactories/packages.config

@MiYanni I don't think we need to add these references to Microsoft.Data.* and System.Spatial as they will get pulled in with WindowsAzure.Storage

The assumption is incorrect. If it is not present in the packages.config, NuGet will not pull those packages. Dependent NuGet packages are not automatically pulled in. Instead, they simply state what other packages are required to be part of the packages.config file.

cormacpayne · 2018-02-12T22:20:47Z

src/ResourceManager/Compute/Stack/Sync/packages.config

@MiYanni I don't think we need to add these references to Microsoft.Data.* and System.Spatial as they will get pulled in with WindowsAzure.Storage

The assumption is incorrect. If it is not present in the packages.config, NuGet will not pull those packages. Dependent NuGet packages are not automatically pulled in. Instead, they simply state what other packages are required to be part of the packages.config file.

cormacpayne · 2018-02-12T22:20:56Z

src/ResourceManager/AzureBatch/Commands.Batch/packages.config

@MiYanni I don't think we need to add these references to Microsoft.Data.* and System.Spatial as they will get pulled in with WindowsAzure.Storage

The assumption is incorrect. If it is not present in the packages.config, NuGet will not pull those packages. Dependent NuGet packages are not automatically pulled in. Instead, they simply state what other packages are required to be part of the packages.config file.

cormacpayne · 2018-02-12T22:21:05Z

src/ResourceManager/ApiManagement/Commands.ApiManagement/packages.config

@MiYanni I don't think we need to add these references to Microsoft.Data.* and System.Spatial as they will get pulled in with WindowsAzure.Storage

The assumption is incorrect. If it is not present in the packages.config, NuGet will not pull those packages. Dependent NuGet packages are not automatically pulled in. Instead, they simply state what other packages are required to be part of the packages.config file.

cormacpayne · 2018-02-12T22:21:25Z

src/ResourceManager/StreamAnalytics/Commands.StreamAnalytics.Test/packages.config

@MiYanni can we remove this?

The .csproj doesn't reference Moq directly. So, it should be fine to remove it from the packages.config.

maddieclayton · 2018-02-13T18:37:28Z

@MiYanni Looks like some merge conflicts have come up - can you resolve?

MiYanni · 2018-02-15T00:27:38Z

@cormacpayne Finally got the solutions/projects working again after additional changes based on your comments (and other findings). Squashed commits. Please re-review.

maddieclayton · 2018-02-15T18:15:01Z

@MiYanni Can you fix the merge conflicts?

MiYanni · 2018-02-15T21:14:02Z

@azuresdkci Retest this please

MiYanni · 2018-02-15T23:38:44Z

On-demand here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/484/

markcowl · 2018-03-01T01:47:19Z

@MiYanni Looks like a merge issue. Can you take a look?

# Conflicts: # setup/azurecmdfiles.wxi

maddieclayton · 2018-03-02T19:30:26Z

On demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/542/

# Conflicts: # src/ResourceManager/Compute/Commands.Compute/packages.config

MiYanni · 2018-03-02T22:48:00Z

On demand here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/545/

# Conflicts: # src/ResourceManager/Storage/Commands.Management.Storage/packages.config

MiYanni · 2018-03-06T18:45:09Z

@azuresdkci Retest this please

MiYanni · 2018-03-06T22:11:23Z

@azuresdkci Retest this please

MiYanni · 2018-03-07T00:06:36Z

@azuresdkci Retest this please

markcowl · 2018-03-08T17:09:20Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/570/

addressed

maddieclayton · 2018-03-08T19:25:44Z

more on demands here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/571/ (and 572)

MiYanni added the Redis Cache label Feb 8, 2018

MiYanni self-assigned this Feb 8, 2018

MiYanni requested review from SiddharthChatrolaMs, cormacpayne and markcowl February 8, 2018 18:37

azuresdkci added the needs-review label Feb 8, 2018

cormacpayne assigned cormacpayne and unassigned MiYanni Feb 12, 2018

cormacpayne previously requested changes Feb 12, 2018

View reviewed changes

cormacpayne added needs-revision and removed Redis Cache needs-review labels Feb 12, 2018

cormacpayne assigned MiYanni and unassigned cormacpayne Feb 12, 2018

maddieclayton added the Merge Conflicts label Feb 13, 2018

MiYanni removed the request for review from SiddharthChatrolaMs February 13, 2018 22:09

MiYanni removed the Merge Conflicts label Feb 13, 2018

MiYanni force-pushed the CommonRefactor branch from 76b42eb to 5d2155b Compare February 15, 2018 00:25

MiYanni assigned cormacpayne and unassigned MiYanni Feb 15, 2018

MiYanni added needs-review and removed needs-revision labels Feb 15, 2018

MiYanni added the Waiting for On Demand label Feb 15, 2018

maddieclayton removed the Waiting for On Demand label Feb 16, 2018

markcowl assigned MiYanni Mar 1, 2018

markcowl added the needs-revision label Mar 1, 2018

MiYanni added 4 commits March 1, 2018 15:13

Merge branch 'preview' into CommonRefactor

7de0d06

# Conflicts: # setup/azurecmdfiles.wxi

Fix merge issue.

9aa3a60

Fix merge issue.

304a181

Fix merge issue.

eb7376b

maddieclayton assigned markcowl and unassigned MiYanni Mar 2, 2018

maddieclayton added needs-review and removed needs-revision labels Mar 2, 2018

maddieclayton added the Waiting for On Demand label Mar 2, 2018

Merge branch 'preview' into CommonRefactor

534c03e

# Conflicts: # src/ResourceManager/Compute/Commands.Compute/packages.config

markcowl added the Do Not Merge 🚫 label Mar 3, 2018

maddieclayton removed Waiting for On Demand needs-review labels Mar 5, 2018

Merge branch 'preview' into CommonRefactor

8af9b59

# Conflicts: # src/ResourceManager/Storage/Commands.Management.Storage/packages.config

markcowl added Waiting for On Demand and removed Do Not Merge 🚫 labels Mar 8, 2018

markcowl removed their assignment Mar 8, 2018

maddieclayton approved these changes Mar 8, 2018

View reviewed changes

maddieclayton merged commit 11e3477 into Azure:preview Mar 8, 2018

Narrow down WindowsAzure.Storage usage #5479

Narrow down WindowsAzure.Storage usage #5479

Uh oh!

Conversation

MiYanni commented Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

MiYanni commented Feb 8, 2018

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maddieclayton commented Feb 13, 2018

Uh oh!

MiYanni commented Feb 15, 2018

Uh oh!

maddieclayton commented Feb 15, 2018

Uh oh!

MiYanni commented Feb 15, 2018

Uh oh!

MiYanni commented Feb 15, 2018

Uh oh!

markcowl commented Mar 1, 2018

Uh oh!

maddieclayton commented Mar 2, 2018

Uh oh!

MiYanni commented Mar 2, 2018

MiYanni commented Feb 8, 2018 •

edited

Loading