Update Contributing.md doc #6368
Conversation
|
@Azure/azure-sdk-eng |
|
@dsgouda @shahabhijeet can you guys please also review and provide feedback on the the documentation update on how to work in this repo. |
42dab89 to
ef282b8
Compare
7825631 to
57891c2
Compare
CONTRIBUTING.md
Outdated
|
|
||
| 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` |
There was a problem hiding this comment.
@shahabhijeet my goal is that we don't need to use msbuild and these targets manually but instead we can simply call dotnet build, dotnet test, and dotnet pack which are the default .NET cli commands. Behind the scenes we can still call those targets but people won't need to know these custom command lines and targets.
|
|
||
| Build tools are now downloaded as part of a nuget package under `root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools` |
There was a problem hiding this comment.
@shahabhijeet once we correctly version and publish this package these steps will no longer be necessary.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
|
||
| > 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 |
There was a problem hiding this comment.
@shahabhijeet we still need to fix the psmodules if we want people to use generate.ps1.
| > 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 | ||
| 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 |
There was a problem hiding this comment.
@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.
| - 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. |
There was a problem hiding this comment.
@shahabhijeet @AlexGhiondea what would be a good alias here?
There was a problem hiding this comment.
We might also want to consider pointing folks at the Codeowners file instead of putting an alias here.
There was a problem hiding this comment.
What do you mean by CodeOwners file?
There was a problem hiding this comment.
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.
57891c2 to
b853df2
Compare
|
|
||
| Build tools are now downloaded as part of a nuget package under `root\restoredPackages\microsoft.internal.netsdkbuild.mgmt.tools` |
There was a problem hiding this comment.
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.
|
|
||
| - `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` |
There was a problem hiding this comment.
add the following
msbuild build.proj /t:Util /p:UtilityName=InstallPsModules
this will install the powershell modules that are needed for using generate.ps1
There was a problem hiding this comment.
This command does not work right now. Error: error MSB4057: The target "Util" does not exist in the project
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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:
- git clean -xdf
- git pull upstream master
- msbuild build.proj /t:Util /p:UtilityName=InstallPsModules
There was a problem hiding this comment.
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?
b853df2 to
2fba2d6
Compare
Done |
2fba2d6 to
6ee1b67
Compare
No description provided.