Update README.md#10
Conversation
|
No issues were found in this pull request. |
|
No issues were found in this pull request. |
| * Open it in Visual Studio and install the latest version of dependent NuGet packages (e.g Microsoft.IdentityModel.Clients.ActiveDirectory) | ||
| * Create (or use and existing) Azure Active Directory Application ID and the corresponding information. If you do not have one and need instructions on how to get one, you can check [here](https://docs.microsoft.com/en-us/rest/api/datacatalog/register-a-client-app) | ||
| * Compile the app. | ||
| * You are now ready to run it. |
There was a problem hiding this comment.
It would be great to at least point them to the file in the project where they can see what the app is doing.
There was a problem hiding this comment.
The app file name in the sample is very descriptive, so that would be a very straightforward thing to be spotted by the developer audience. Hence, resolving as By Design.
There was a problem hiding this comment.
Please help me understand how the dev is harmed by a sentence of the form "To use the sample please open the Program.cs file and examine the contents to see how the REST APIs are used."?
| * Download the sample project | ||
| * Open it in Visual Studio and install the latest version of dependent NuGet packages (e.g Microsoft.IdentityModel.Clients.ActiveDirectory) | ||
| * Create (or use and existing) Azure Active Directory Application ID and the corresponding information. If you do not have one and need instructions on how to get one, you can check [here](https://docs.microsoft.com/en-us/rest/api/datacatalog/register-a-client-app) | ||
| * Compile the app. |
There was a problem hiding this comment.
I would use Visual Studio and give the commands step by step. So you would specify the compile command to use. What file did they open to start the compile? What compile command should they use?
|
|
||
| * Download the sample project | ||
| * Open it in Visual Studio and install the latest version of dependent NuGet packages (e.g Microsoft.IdentityModel.Clients.ActiveDirectory) | ||
| * Create (or use and existing) Azure Active Directory Application ID and the corresponding information. If you do not have one and need instructions on how to get one, you can check [here](https://docs.microsoft.com/en-us/rest/api/datacatalog/register-a-client-app) |
There was a problem hiding this comment.
/or use and existing/or use an existing/
/and the corresponding information./and corresponding information./ - But my suggestion is that you remove 'corresponding information' all together and instead be specific. What exactly do they need?
There was a problem hiding this comment.
#1 fixed
#2 fixed the typo, however regarding specifying the actual info in details, my point here is to give the users a starting point on what steps required to create Client ID and in case they need more info, they can refer to the docs referenced above. the goal for the README here is to outline the steps at high-level along with pointers to the docs in case more details needed to avoid overloading the README.
There was a problem hiding this comment.
I'm not asking you to give them step by step instructions (although that isn't a bad idea, good ReadMes try hard to be 100% self contained, that's a best practice since it substantially increases the probability that the reader will successfully get the project running). But the text says "the corresponding information" which is vague and so creates difficulty for the user who no longer knows what they are supposed to find in the link. So best practice is to be specific, in addition to the App ID what specific other information do they need? Again, you aren't on the hook for defining how they get it, just for defining what it is.
There was a problem hiding this comment.
With any sample our goal is always, I believe, to get the developer up and running as quickly as possible. The fastest way to lose a dev's attention is to force them to read through a bunch of links and do a bunch of research just to get some code up and running. What every dev I've ever worked with has wanted and what I wanted as a dev is to first quickly get the project up and running to see if it's worth anything. Once the project is running and if it looks useful then and only then will more research be done. So typically the goal of any readme is to get the dev on the shortest possible path between cloning the project and getting it running. This is why good readmes will try to include all the specific commands necessary to successfully use the project. The less the dev has to think the sooner they get the project running. In fact the desire to get devs running fast is what led to the invention of tools like NuGet and NPM. To reduce the path between "desire to run code" and "code running".
Let's use a real world example. A very popular open source project is Electron. Please head over to its readme file. https://github.com/electron/electron What do you see? It gives the NPM command line to do the install (in our case this is exactly the equivalent of the clone command I suggested). Then it gives four command line instructions to get it going. No long article. No pointers. Just five commands and it's running. And that is for a vastly more complex project than this.
Or you can look at VSCode. They take a slightly different approach since they really want you to install the prebuilt binary. But if you go their instructions for building the project yourself at https://github.com/Microsoft/vscode/wiki/How-to-Contribute#build-and-run-from-source what do you see? First they give the exact clone command. Then they give the exact command line strings needed to do the build.
I would humbly suggest that the sophistication of a developer who is going to build electron or VSCode is pretty high yet they still include the clone commands and they still include the command line strings. I believe we should emulate their behavior.
| To get started using this sample, you first need to build the executable, following below steps: | ||
|
|
||
| * Download the sample project | ||
| * Open it in Visual Studio and install the latest version of dependent NuGet packages (e.g Microsoft.IdentityModel.Clients.ActiveDirectory) |
There was a problem hiding this comment.
/Open it in Visual Studio/Open the sample project in Visual Studio/ - It's usually better to be specific rather than general, e.g. bind the object rather than using 'it'.
/of dependent/of the dependent/
It would also be good to give the specific command for installing the NuGet packages. With ReadMes the lest thinking required the fewer support requests one gets. So I usual aim to provide something as close to possible as the actual command line commands as possible.
There was a problem hiding this comment.
Point 1 & 2 are resolved
Point 3 is By Design as the sample code is well annotated describing how to install the dependent NuGet packages.
(e.g // run "Install-Package Microsoft.IdentityModel.Clients.ActiveDirectory" from the NuGet Package Manager Console.)
There was a problem hiding this comment.
Let's compare.
Path 1 - The dev is warned right here and provided in the readme with the appropriate command to fix the NuGet issue before they compile and encounter any errors
Path 2 - The dev tries to build the project. Fails because of some dependency issue. And is then expected to go find the right file which happens to contain the correct command to figure out where the notice about the fix is.
I am struggling to understand how path 2 can be considered superior to path 1. Why would we intentionally put blocks in the way of the dev getting quickly started?
|
|
||
| To get started using this sample, you first need to build the executable, following below steps: | ||
|
|
||
| * Download the sample project |
There was a problem hiding this comment.
It would be better to use numbers here instead of *s. That makes is clear that there is a series of specific steps that are to be performed in the prescribed order.
| This sample shows you how to Register, Search, and Delete a data asset using the Data Catalog REST API. | ||
| This sample shows you how to use the Azure Data Catalog REST API to Register, Search, and Delete data assets. | ||
|
|
||
| To get started using this sample, you first need to build the executable, following below steps: |
There was a problem hiding this comment.
/you first need to build the executable, following below steps:/you first need to build the executable following the steps below:/
|
No issues were found in this pull request. |
|
No issues were found in this pull request. |
|
No issues were found in this pull request. |
Adding additional guidance on how to Create a Client App and register it in Azure Active Directory (AAD)