Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Update README.md #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update README.md #5

wants to merge 3 commits into from

Conversation

SaraMS
Copy link

@SaraMS SaraMS commented Jun 12, 2018

Adding guidance on installing the updated NuGet packages

@acomsmpbot
Copy link
Contributor

No issues were found in this pull request.

@acomsmpbot
Copy link
Contributor

No issues were found in this pull request.

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
Member

Choose a reason for hiding this comment

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

Please see my comments the equivalent to lines 13-16 above in Azure-Samples/data-catalog-dotnet-get-started#10

Copy link
Author

Choose a reason for hiding this comment

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

Resolved.

README.md Outdated
This sample shows you how to bulk register and annotate data assets from an Excel workbook using the Data Catalog REST API and Open XML.

To get started using this sample, you first need to build the executable, following below steps:
* Download the sample project
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful to give the GIT command they can use to do that.

Copy link
Author

Choose a reason for hiding this comment

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

This is By Design. I feel this would be very basic for the developer audience here. I'm also following the practice used in https://azure.microsoft.com/en-us/resources/samples/data-catalog-dotnet-import-export/

Copy link
Member

Choose a reason for hiding this comment

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

Many enterprise developers never work with Git or work with a single repo that they don't have to clone more than once. So by not adding a single line we force the enterprise dev to wonder "Um... how do I do that download? What was the command line for that? Oh wait can GitHub do it for me? I don't remember!" or we can add "git clone https://github.com/Azure-Samples/data-catalog-dotnet-excel-register-data-assets.git" and be done with it. Please help me understand what harm it would do to include this sentence and how that harm would outweigh the benefits?

README.md Outdated
This sample shows you how to register and annotate data assets from an Excel workbook.
This sample shows you how to bulk register and annotate data assets from an Excel workbook using the Data Catalog REST API and Open XML.

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

Choose a reason for hiding this comment

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

/sample, you/sample you/ - No comma because there aren't two separate thoughts.

/executable,/executable/ - Again, no command. Thoughts aren't being separated.

Copy link
Author

Choose a reason for hiding this comment

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

Resolved.

README.md Outdated
@@ -6,4 +6,11 @@ author: dvana

# Azure Data Catalog - Bulk register and annotate

This sample shows you how to register and annotate data assets from an Excel workbook.
This sample shows you how to bulk register and annotate data assets from an Excel workbook using the Data Catalog REST API and Open XML.
Copy link
Member

Choose a reason for hiding this comment

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

/the Data Catalog/the Azure Data Catalog/

Copy link
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
Contributor

No issues were found in this pull request.

@SaraMS SaraMS closed this Jul 9, 2018
@SaraMS SaraMS reopened this Jul 9, 2018
@acomsmpbot
Copy link
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.

None yet

3 participants