-
Notifications
You must be signed in to change notification settings - Fork 251
Update README.md - ai branch #34
base: ai
Are you sure you want to change the base?
Conversation
Updated the ReadMe to reflect the specific steps required to configure AI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some general comments and requesting change on version number to 2.0.0-BETA and later on [2.0.,) once stable release is out,
README.md
Outdated
* [Deploy to Azure Container Service Kubernetes cluster using Maven plugin](./doc/deployment/deploy-to-azure-container-service-using-maven-plugin.md) | ||
* [Deploy to Azure Web App for Containers using Maven plugin](./doc/deployment/deploy-to-azure-web-app-using-maven-plugin.md) | ||
* [Useful link](#useful-link) | ||
This TodoList app is an Azure Java application. It provides end-to-end CRUD operation to todo list item from front-end AngularJS code. Behind the scene, todo list item data store is [Azure CosmosDB DocumentDB](https://docs.microsoft.com/en-us/azure/cosmos-db/documentdb-introduction). This application uses [Azure CosmosDB DocumentDB Spring Boot Starter](https://github.com/Microsoft/azure-spring-boot/tree/master/azure-starters/azure-documentdb-spring-boot-starter) and AngularJS to interact with Azure. In particular, this version of the application shows how to use [Azure Application Insights](https://docs.microsoft.com/en-us/azure/application-insights/app-insights-java-get-started) to monitor the performance and usage of the application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we still point to https://docs.microsoft.com/en-us/azure/application-insights/app-insights-java-get-started. as it doesn't have instructions on configuration with SpringBoot. Might be customers get mislead? Just brainstorming, if answer is no - where should we point them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder - I meant to ask Asir about whether there are plans to improve this documentation to mention SpringBoot. Also, I clarified in the ReadMe that this sample shows SpringBoot and that for other examples, refer to the 'getting started' docs. Let me know if you still think this is unclear.
README.md
Outdated
<dependency> | ||
<groupId>com.microsoft.azure</groupId> | ||
<artifactId>applicationinsights-web</artifactId> | ||
<version>[1.0,)</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update version to 2.0.0-BETA for time being. The [1.0.,) will pull the last 1.x version which won't be the latest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
README.md
Outdated
* Follow the steps for creating the Azure App Insights resource (as shown in the Requirements section) and copy the instrumentation key. | ||
Paste the instrumentation key into the 'src/main/resources/ApplicationInsights.xml' file and save it. | ||
|
||
<!-- The key from the portal: --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
following markdown code snippet syntax. like
<- The key from the Portal-->
your configuration sniipet
README.md
Outdated
|
||
* In addition, you may notice that the following has already been configured for you to setup App Insights: | ||
1. The dependency to Application Insights has been added to the pom.xml: | ||
<dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, please follow above markdown code snippet syntax
README.md
Outdated
|
||
## Requirements | ||
|
||
* [JDK](http://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html) 1.8 and above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those steps are still necessary for user to finish a complete end to end todo app with application insight. please add steps of application insight into original steps, instead of overwrite it. thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that I don't duplicate all the steps, I simply added a link to the master's ReadMe for reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think link to master is fine. It doesn't make sense to add to much text in one file. Ultimately it's your team's decision on it. I am fine with both, personally would prefer less text and duplication.
Updated the ReadMe to reflect the specific steps required to configure AI.