Skip to content
This repository was archived by the owner on Oct 12, 2023. It is now read-only.

Update README.md#10

Open
SaraMS wants to merge 5 commits into
Azure-Samples:masterfrom
SaraMS:patch-1
Open

Update README.md#10
SaraMS wants to merge 5 commits into
Azure-Samples:masterfrom
SaraMS:patch-1

Conversation

@SaraMS
Copy link
Copy Markdown

@SaraMS SaraMS commented Jun 11, 2018

Adding additional guidance on how to Create a Client App and register it in Azure Active Directory (AAD)

@acomsmpbot
Copy link
Copy Markdown
Contributor

No issues were found in this pull request.

@acomsmpbot
Copy link
Copy Markdown
Contributor

No issues were found in this pull request.

Comment thread README.md Outdated
* 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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be great to at least point them to the file in the project where they can see what the app is doing.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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."?

Comment thread README.md Outdated
* 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.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

?

Comment thread README.md Outdated

* 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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

#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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Comment thread README.md Outdated
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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/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.

Copy link
Copy Markdown
Author

@SaraMS SaraMS Jul 9, 2018

Choose a reason for hiding this comment

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

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.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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?

Comment thread README.md Outdated

To get started using this sample, you first need to build the executable, following below steps:

* Download the sample project
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved.

Comment thread README.md Outdated
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:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/you first need to build the executable, following below steps:/you first need to build the executable following the steps below:/

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved

@acomsmpbot
Copy link
Copy Markdown
Contributor

No issues were found in this pull request.

@acomsmpbot
Copy link
Copy Markdown
Contributor

No issues were found in this pull request.

@acomsmpbot
Copy link
Copy Markdown
Contributor

No issues were found in this pull request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants