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.

Sign up for GitHub

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

Update Contributing.md doc #6368

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Update Contributing.md doc #6368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
267 changes: 165 additions & 102 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,106 +1,179 @@
## Contributing

| Component | Build Status |
| --------- | ------------ |
| Management Libraries | [![Build Status](https://travis-ci.org/Azure/azure-sdk-for-net.svg?branch=master)](https://travis-ci.org/Azure/azure-sdk-for-net) |
| Client Libraries | [![Build Status](https://dev.azure.com/azure-sdk/public/_apis/build/status/41?branchName=master)](https://dev.azure.com/azure-sdk/public/_build/latest?definitionId=41&branchName=master) |

### Prerequisites:
Install VS 2017 (Professional or higher) and make sure you have the latest updates (https://www.visualstudio.com/).

### To build:
=======

#### If you are building from VS, add a nuget feed source that points to < root >\tools\LocalNugetFeed directory
1. Open any solution, eg "Compute\Microsoft.Azure.Management.Compute.sln"
2. Build solution from VS

#### Full Build from command line

1. Open VS 2017 developer command prompt
2. Navigate to repository root directory
3. Invoke **msbuild** build.proj /t:Build
will Build
##### *Build* without any scope will build all management SDK's.

#### Build single SDK
`msbuild build.proj /t:CreateNugetPackage /p:scope=Compute`

### To run tests:
Using Visual Studio:
- Build project in VS
- "Test Explorer" window will get populated with tests. Right click on a test that you wish to Run/Debug.

Using the command line:
msbuild .\build.proj /t:Runtests /p:Scope=Compute
in the above example RunTests will build and run tests for Compute only
or you can use command line CLI
dotnet test Compute\Microsoft.Azure.Management.Compute\tests\Microsoft.Azure.Management.Tests.csproj

#### Create single nuget package
# Contributing

| Component | Build Status |
| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Management Libraries | [![Build Status](https://dev.azure.com/azure-sdk/public/_apis/build/status/net/net%20-%20mgmt%20-%20ci?branchName=master)](https://dev.azure.com/azure-sdk/public/_build/latest?definitionId=529&branchName=master) |
| Client Libraries | [![Build Status](https://dev.azure.com/azure-sdk/public/_apis/build/status/41?branchName=master)](https://dev.azure.com/azure-sdk/public/_build/latest?definitionId=41&branchName=master) |

# Prerequisites:

Install VS 2019 (Professional or higher) and make sure you have the latest updates (https://www.visualstudio.com/).

**Client Libraries** are sdks used to interact with azure resources at application run time while **Management Libraries** are those used to manage (create/modify/delete) Azure resources.
<br/><br/>

# Management Libraries

## TO BUILD:

1. Open any solution, eg "SDKs\Compute\Microsoft.Azure.Management.Compute.sln"
2. Build solution from Visual Studio

### Single Service from Command Line

1. Open VS 2019 command Prompt
2. From the root directory
3. Invoke `msbuild eng\mgmt.proj /p:scope=Compute`

> _Build_ without any scope will build all management SDK's.

### Create single nuget package

In order to build one package and run it's test
`msbuild build.proj /t:CreateNugetPackage /p:scope=Compute`
`msbuild eng\mgmt.proj /t:CreateNugetPackage /p:scope=Compute`
Nuget package will be created in root directory under \artifacts\packages\Debug (default configuration is Debug)

## TO TEST:

### Using Visual Studio:

1. Build project in Visual Studio.
2. **Test Explorer** window will get populated with tests. Select test and `Run` or `Debug`

### Using the command line:

Run e.g. `msbuild eng\mgmt.proj /t:"Runtests" /p:Scope=Compute`
In the above example _RunTests_ will build and run tests for Compute only or you can use command line CLI
`dotnet test Compute\Microsoft.Azure.Management.Compute\tests\Microsoft.Azure.Management.Tests.csproj`

### Non-Windows command line build
Now you can use the same command on non-windows as above
for e.g. on Ubuntu you can do something like below:
`dotnet msbuild build.proj /t:Build /p:scope=Compute`
`dotnet msbuild build.proj /t:RunTests /p:scope=Compute`
`dotnet msbuild build.proj /t:CreateNugetPackage /p:scope=Compute`

Now you can use the same command on non-windows as above for e.g. on Ubuntu you can do something like below:

- `dotnet msbuild eng\mgmt.proj /t:Build /p:scope=Compute`
- `dotnet msbuild eng\mgmt.proj /t:RunTests /p:scope=Compute`
- `dotnet msbuild eng\mgmt.proj /t:CreateNugetPackage /p:scope=Compute`
Copy link
Member

Choose a reason for hiding this comment

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

add the following
msbuild build.proj /t:Util /p:UtilityName=InstallPsModules
this will install the powershell modules that are needed for using generate.ps1

Copy link
Member Author

Choose a reason for hiding this comment

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

This command does not work right now. Error: error MSB4057: The target "Util" does not exist in the project

Copy link
Member

Choose a reason for hiding this comment

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

As we chatted I think we should hide that command completely and instead we should have a helper script under eng to handle this setup and then update all the generate.ps1 to call that init script.

Copy link
Member

Choose a reason for hiding this comment

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

@weshaggard changing generate.ps1 for every RP is a bigger change (at least I think that is little more involved as it will require all generate.ps1 to be updated and tested)
So unless we are planning to do that change right now, we need to have a way for RPs to get the updated scripts documented, because if we do not do that, partners will start adding their own custom scripts in the repo and that will be even bigger workitem for a clean up.

@chidozieononiwu our partners have started using that target to get updated powershell scripts, and I am not able to reproduce the error you are mentioning.
The only way you end up getting that error if you are still using old tools.
let me know if you still see the issue if you do the following:

  1. git clean -xdf
  2. git pull upstream master
  3. msbuild build.proj /t:Util /p:UtilityName=InstallPsModules

Copy link
Member

Choose a reason for hiding this comment

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

While the change might be rather large, with a lot of files generate.ps1 files changed I don't think you need to test everyone just a few to verify it works. As it should be as simple as adding one line to the top of all generate.ps1. If you don't think that is worth the effort right now might I suggest you go ahead and create a simple wrapper script in eng/mgmt that people can call instead of trying to have them remember and type this fairly complicated msbuild command?

- `dotnet msbuild build.proj /t:Util /p:UtilityName=InstallPsModules`

### Update build tools
Build tools are now downloaded as part of a nuget package under root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools
If for any reason there is an update to the build tools, you will then need to first delete directory root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools and reexecute your build command. This will simply get the latest version of build tools.

Build tools are now downloaded as part of a nuget package under `root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools`
Copy link
Member

Choose a reason for hiding this comment

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

@shahabhijeet once we correctly version and publish this package these steps will no longer be necessary.

Copy link
Member

Choose a reason for hiding this comment

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

That is correct.
The only thing that needs to be added is, for getting latest version, you will have to pull latest from upstream and then execute build targets.
But yes once we version and start publishing nuget pacakges to nuget feed, no need to delete anything.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to add any steps telling people to pull from latest upstream. They will automatically have the latest tools if they are on the latest source.

If for any reason there is an update to the build tools, you will then need to first delete directory `root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools` and re-execute your build command. This will simply get the latest version of build tools.

# Client Libraries

## TO BUILD:

### Single Service from Command Line

1. Open VS 2019 Command Prompt
2. Navigate to service directory e.g. _"sdk\eventhub"_
3. Invoke `dotnet build`
4. or Build the **service.proj** in the repo root, passing the directory name of the specific service as a property. e.g. `dotnet build eng\service.proj /p:ServiceDirectory=eventhub`

### Single Service from Visual Studio

## To on-board new libraries
1. Open any data-plane solution e.g. _"sdk\eventhub\Microsoft.Azure.EventHubs.sln"_
2. Build solution from Visual Studio

### All Client Services from Command Line

1. Open VS 2019 Command Prompt
2. Navigate to repository root directory
3. Invoke `dotnet build eng\service.proj`

## TO TEST:

### Single Service from Command Line

1. Open VS 2019 Command Prompt
2. Navigate to service directory e.g. _"sdk\eventhub"_
3. Invoke `dotnet test --filter TestCategory!=Live` _(Skips live tests)_
4. or run test against **service.proj** in the repo root, passing the directory name of the specific service as a property. e.g. `dotnet test eng\service.proj /p:ServiceDirectory=eventhub --filter TestCategory!=Live`

### Single Service from Visual Studio

1. Build.
2. Test Explorer window will get populated with tests. Select test and `Run` or `Debug`

### All Client Services from Command Line

1. Open VS 2019 Command Propmpt
2. Navigate to repository root directory
3. Invoke `dotnet test eng\service.proj --filter TestCategory!=Live`
<br/><br/>

# On-boarding New Libraries

### Project Structure

In "sdk\< Service Name >", you will find projects for services that have already been implemented
In `sdk\< Service Name >`, you will find projects for services that have already been implemented

- The easiest way is to model your test/sdk project using existing test/sdk project
- Make a copy of existing csproj and change the meta data relevant to your SDK
- This ensures the SDK that will be generated will be consistent (including consistent target frameworks)
- Each service contains a project for their generated/customized code
- The folder 'Generated' contains the generated code
- The folder 'Customizations' contains additions to the generated code - this can include additions to the generated partial classes, or additional classes that augment the SDK or
1. Client library projects needs to use the $(RequiredTargetFrameworks) *(defined in eng/Directory.Build.Data.props)* property in its TargetFramework while management library projects should use $(SdkTargetFx) _(defined in AzSdk.reference.props)_
2. Projects of related packages are grouped together in a folder following the structure specified in [Repo Structure](https://github.com/Azure/azure-sdk/blob/master/docs/engineering-system/repo-structure.md)
- Client library packages are in a folder name like **_Microsoft.Azure.< ServiceName >_**
- Management library packages are in a folder named like **_Microsoft.Azure.Management.< Resource Provider Name >_**
3. Each shipping package contains a project for their **generated** and /or **Customization** code
- The folder **'Generated'** contains the generated code
- The folder **'Customizations'** contains additions to the generated code - this can include additions to the generated partial classes, or additional classes that augment the SDK or call the generated code
- The file **generate.cmd**, used to generate library code for the given package, can also be found in this project

### Standard Process

1. Create fork of [Azure REST API Specs](https://github.com/azure/azure-rest-api-specs)
2. Create fork of [Azure SDK for .NET](https://github.com/azure/azure-sdk-for-net)
3. Create your Swagger specification for your HTTP API. For more information see
[Introduction to Swagger - The World's Most Popular Framework for APIs](http://swagger.io)
4. Install the latest version of AutoRest and use it to generate your C# client. For more info on getting started with AutoRest,
see the [AutoRest repository](https://github.com/Azure/autorest)
5. Create a branch in your fork of Azure SDK for .NET and add your newly generated code to your project. If you don't have a project in the SDK yet, look at some of the existing projects and build one like the others.
6. **MANDATORY**: Add or update tests for the newly generated code.
7. Once added to the Azure SDK for .NET, build your local package using command
e.g.
`msbuild build.proj /t:CreateNugetPackage /p:scope=Compute`
8. A Pull request of your Azure SDK for .NET changes against **master** branch of the [Azure SDK for .NET](https://github.com/azure/azure-sdk-for-net)
9. Both the pull requests will be reviewed and merged by the Azure SDK team
1. Create fork of [Azure REST API Specs](https://github.com/azure/azure-rest-api-specs)
2. Create fork of [Azure SDK for .NET](https://github.com/azure/azure-sdk-for-net)
3. Create your Swagger specification for your HTTP API. For more information see
[Introduction to Swagger - The World's Most Popular Framework for APIs](http://swagger.io)
4. Install the latest version of AutoRest and use it to generate your C# client. For more info on getting started with AutoRest,
see the [AutoRest repository](https://github.com/Azure/autorest)
5. Create a branch in your fork of Azure SDK for .NET and add your newly generated code to your project. If you don't have a project in the SDK yet, look at some of the existing projects and build one like the others.
6. **MANDATORY**: Add or update tests for the newly generated code.
7. Once added to the Azure SDK for .NET, build your local package using [client](#client-libraries) or [management](#management-libraries) library instructions shown in the above sections.
8. A Pull request of your Azure SDK for .NET changes against **master** branch of the [Azure SDK for .NET](https://github.com/azure/azure-sdk-for-net)
9. The pull requests will be reviewed and merged by the Azure SDK team

### New Resource Provider

1. If you have never created an SDK for your service before, you will need the following things to get your SDK in the repo
2. Follow the standard process described above.
3. Directory names helps in using basic heuristics in finding projects as well it's associated test projects during CI process.
4. Create a new directory (name of your service e.g. Compute, Storage etc)
6. For e.g. for management plane SDKs the following directory structure
- `<RPName>\<packageName>\src\Microsoft.Azure.Management.<RPName>\Microsoft.Azure.Management.<RPName>.csproj`
- `<RPName>\<packageName>\tests\Microsoft.Azure.Management.<RPName>.Tests.csproj`
3. Project names helps in using basic heuristics in finding projects as well it's associated test projects during CI process.
4. Create a new directory using the name of your service as specified in [azure-rest-api-specs/specification](https://github.com/Azure/azure-rest-api-specs/tree/master/specification) Repo
5. Follow the the directory structure below

```
sdk\<service name>\<package name>\README.md
sdk\<service name>\<package name>\*src*
sdk\<service name>\<package name>\*tests*
sdk\<service name>\<package name>\*samples*
```

e.g.

```
sdk\eventgrid\Microsoft.Azure.EventGrid\src\Microsoft.Azure.EventGrid.csproj
sdk\eventgrid\Microsoft.Azure.EventGrid\tests\Microsoft.Azure.EventGrid.Tests.csproj
sdk\eventgrid\Microsoft.Azure.Management.EventGrid\src\Microsoft.Azure.Management.EventGrid.csproj
sdk\eventgrid\Microsoft.Azure.Management.EventGrid\tests\Microsoft.Azure.Management.EventGrid.Tests.csproj
```

> \*Ensure that your service name is the same as it is specified in the [azure-rest-api-specs/specification](https://github.com/Azure/azure-rest-api-specs/tree/master/specification) Repo, that your csproj files starts with **Microsoft.Azure\***
> , that test files end with **.Tests** and that management plane project files contain **.Management.**

7. Copy .csproj from any other .csproj and update the following information in the new .csproj
- PackageId
- Description
- AssemblyTitle
- AssemblyName
- Version
- PackageTags
- PackageReleaseNotes (this is important because this information is displayed on www.nuget.org when your nuget package is published
8. Copy existing generate.ps1 file from another dirctory and update the `ResourceProvider` name that is applicable to your SDK. Resource provider refers to the relative path of your REST spec directory in Azure-Rest-Api-Specs repository
During SDK generation, this path helps to locate the REST API spec from the `https://github.com/Azure/azure-rest-api-specs`

| Project Properties |
| ------------------- |
| AssemblyTitle |
| Description |
| VersionPrefix |
| PackageTags |
| PackageReleaseNotes |
| |

> PackageReleaseNotes are important because this information is displayed on www.nuget.org when your nuget package is published

8. Copy existing generate.ps1 file from another service and update the `ResourceProvider` name that is applicable to your SDK. Resource provider refers to the relative path of your REST spec directory in Azure-Rest-Api-Specs repository
Copy link
Member

Choose a reason for hiding this comment

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

@shahabhijeet we still need to fix the psmodules if we want people to use generate.ps1.

During SDK generation, this path helps to locate the REST API spec from the `https://github.com/Azure/azure-rest-api-specs`

### Code Review Process
Copy link
Member

Choose a reason for hiding this comment

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

@shahabhijeet @AlexGhiondea can you guys look through this and ensure it matches the expectations across both teams? I'm a little worried about things like SLA of 48 hours for example.


Expand All @@ -123,8 +196,8 @@ Before a pull request will be considered by the Azure SDK team, the following re
- Short description of the payload of pull request.
- After the pull request is submitted:
- Our SLA is 48 hours. When your PR is submitted someone on our team will be auto assigned the PR for review. No need to email us
- MS internal folks, please reach out to us via our slack channel or
- Send an email to the Azure DevEx .Net team (azdevxdotnet@microsoft.com) alias.
- MS internal folks, please reach out to us via our Teams channel or
- Send an email to the Azure Developer Platform team [adpteam@microsoft.com](mailto:adpteam@microsoft.com) alias.
Copy link
Member

Choose a reason for hiding this comment

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

@shahabhijeet @AlexGhiondea what would be a good alias here?

Copy link
Member

Choose a reason for hiding this comment

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

We might also want to consider pointing folks at the Codeowners file instead of putting an alias here.

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean by CodeOwners file?

Copy link
Member

Choose a reason for hiding this comment

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

We have plans to setup CODEOWNERS files in all our repos (see https://help.github.com/en/articles/about-code-owners), once that happens we should just refer to that file to find the correct owner to contact.

- Include all interested parties from your team as well.
- In the message, make sure to acknowledge that the legal signoff process is complete.

Expand All @@ -135,26 +208,16 @@ Once all of the above steps are met, the following process will be followed:
- The owner will then respond to the email sent as part of the pull request, informing the group of the completion of the request.
- If the request does not meet any of the requirements, the pull request will not be merged, and the necessary fixes for acceptance will be communicated back to the partner team.

### Adding Tests

Regarding the test project, one thing that's important is to name the test project by adding a ".Tests" suffix to the folder name for the folder containing your project. For example, the test project for "Compute\Management.Compute" should be named 'Compute.Tests'

- This is for improving CI performance so to find exactly one copy of your test assembly.
- Also, due to test dependencies, the test project should build both .NET 4.5.2 and NETStandard 1.4. For example, check out "src\SDKs\Resource\Resource.tests"

### Client Library Tested OSs and .NET Versions

| | Linux (Ubuntu 16.04) | MacOS 10.13 | Windows Server 2016 |
|------------------------|----------------------|-------------|---------------------|
| **.NET Core 2.1** | x | x | x |

| | Linux (Ubuntu 16.04) | MacOS 10.13 | Windows Server 2016 |
| ----------------- | -------------------- | ----------- | ------------------- |
| **.NET Core 2.1** | x | x | x |

### Issues with Generated Code

Much of the SDK code is generated from metadata specs about the REST APIs. Do not submit PRs that modify generated code. Instead,
- File an issue describing the problem,
- Refer to the the [AutoRest project](https://github.com/azure/autorest) to view and modify the generator, or
- Add additional methods, properties, and overloads to the SDK by adding classes in the 'Customizations' folder of a project

Much of the management plane SDK code is generated from metadata specs about the REST APIs. Do not submit PRs that modify generated code. Instead,

![Impressions](https://azure-sdk-impressions.azurewebsites.net/api/impressions/azure-sdk-for-net%2FREADME.png)
- File an issue describing the problem,
- Refer to the the [AutoRest project](https://github.com/azure/autorest) to view and modify the generator, or
- Add additional methods, properties, and overloads to the SDK by adding classes in the 'Customizations' folder of a project
16 changes: 9 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ You can find NuGet packages for these libraries [here](packages.md).

## Getting started

To get started with a specific library, see the **README.md** file located in the library's project folder.
To get started with a specific library, see the **README.md** file located in the library's project folder.
The following sections provide direct links to READMEs of the most commonly used libraries.

### Core services

* [Azure.Messaging.EventHubs](/sdk/eventhub/Azure.Messaging.EventHubs/README.md)
* [Azure.Identity.KeyVault.Keys](/sdk/keyvault/Azure.Security.KeyVault.Keys/Readme.md)
* [Azure.Identity.KeyVault.Secrets](/sdk/keyvault/Azure.Security.KeyVault.Secrets/Readme.md)
* [Azure.Storage.Blobs](/sdk/storage/Azure.Storage.Blobs/README.md)
* [Azure.Storage.Files](/sdk/storage/Azure.Storage.Files/README.md)
* [Azure.Storage.Queues](/sdk/storage/Azure.Storage.Queues/README.md)
- [Azure.Messaging.EventHubs](/sdk/eventhub/Azure.Messaging.EventHubs/README.md)
- [Azure.Identity.KeyVault.Keys](/sdk/keyvault/Azure.Security.KeyVault.Keys/Readme.md)
- [Azure.Identity.KeyVault.Secrets](/sdk/keyvault/Azure.Security.KeyVault.Secrets/Readme.md)
- [Azure.Storage.Blobs](/sdk/storage/Azure.Storage.Blobs/README.md)
- [Azure.Storage.Files](/sdk/storage/Azure.Storage.Files/README.md)
- [Azure.Storage.Queues](/sdk/storage/Azure.Storage.Queues/README.md)

### Shared libraries

Expand All @@ -36,3 +36,5 @@ Azure SDK clients use shared libraries implementing retries, logging, transport
## Contributing

For details on contributing to this repository, see the [contributing guide](CONTRIBUTING.md).

![Impressions](https://azure-sdk-impressions.azurewebsites.net/api/impressions/azure-sdk-for-net%2FREADME.png)