-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improving dialog-based acquireToken javadocs #1268
Conversation
* AUTO. | ||
* | ||
* acquireToken will authorize an end user to call the specified resource. | ||
* The minted access token will be sent to the {@link AuthenticationCallback} |
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.
Note about clarity: "minted" may confuse users into thinking that our library creates access tokens when, in reality, they are given to us.
How about:
- "The access token returned from the STS..."
- "The acquired access token...."
- "The access token"
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.
Good call out. Fixed.
adal/src/main/java/com/microsoft/aad/adal/AuthenticationContext.java
Outdated
Show resolved
Hide resolved
* and can be used to call the specified protected resource. | ||
* By default, acquireToken will attempt to fulfill the request silently, and | ||
* perform an interactive login if needed or explicitly specified in | ||
* the request. This overload uses an |
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.
There are two-spaces after the period in the javadoc. Just a formatting thing.
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.
Bad habit :)
adal/src/main/java/com/microsoft/aad/adal/AuthenticationContext.java
Outdated
Show resolved
Hide resolved
adal/src/main/java/com/microsoft/aad/adal/AuthenticationContext.java
Outdated
Show resolved
Hide resolved
adal/src/main/java/com/microsoft/aad/adal/AuthenticationContext.java
Outdated
Show resolved
Hide resolved
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.
Few questions, suggested edits
Issue: #1267
Improving dialog javadocs by explaining usage, limitations (in this case these overloads do not work with brokered flows), and overall improve the diction.