[Communication] - Documentation - Update docs #730

JoshuaLai · 2021-02-23T19:59:49Z

No description provided.

sacheu · 2021-02-23T20:03:04Z

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift

 */
 internal class AutoRefreshTokenCredential: CommunicationTokenCredentialProviding {
    private let accessTokenCache: ThreadSafeRefreshableAccessTokenCache

    /**
-     Creates a `CommunicationTokenCredential` that automatically refreshes the token using the provided `tokenRefresher`.
+     Creates a `CommunicationTokenCredential` that automatically refreshes
+     the token using the provided `tokenRefresher`.


Why we break this into 2 lines?

sacheu · 2021-02-23T20:03:57Z

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift

@@ -60,10 +62,12 @@ internal class AutoRefreshTokenCredential: CommunicationTokenCredentialProviding
    }

    /**
-     Retrieve an access token from the cache, or from the `tokenRefresher` if the token is not in the cache or is expired.
+     Retrieve an access token from the cache, or from the `tokenRefresher`
+     if the token is not in the cache or is expired.


sacheu · 2021-02-23T20:04:08Z

sdk/communication/AzureCommunication/Source/Authentication/CommunicationAccessToken.swift

-
+    /**
+     Creates a new instance of CommunicationAccessToken using the provided `token`
+     and `expiresOn`.


Same line as above?

sacheu · 2021-02-23T20:05:13Z

sdk/communication/AzureCommunication/Source/Identifiers.swift

@@ -27,7 +27,9 @@
 import Foundation

 /**
- Common Communication Identifier protocol for all Azure Communication Services. All Communication Identifiers conform to this protocol.
+ Common Communication Identifier protocol for all
+ Azure Communication Services.


and seems like '. All Communication Identifiers conform to this protocol.' is removed.

sacheu · 2021-02-23T20:21:22Z

sdk/communication/AzureCommunication/Source/Identifiers.swift

-     - Parameter userId: Id of the Microsoft Teams user. If the user isn't anonymous, the id is the AAD object id of the user.
-     - Parameter isAnonymous: Set this to true if the user is anonymous, for example when joining a meeting with a share link.
+     - Parameter userId: Id of the Microsoft Teams user. If the user isn't anonymous,
+     the id is the AAD object id of the user.


Putting this in separate lines make them harder to read

sacheu · 2021-02-23T20:21:55Z

sdk/communication/AzureCommunication/Source/Identifiers.swift

+     - Parameter userId: Id of the Microsoft Teams user. If the user isn't anonymous,
+     the id is the AAD object id of the user.
+     - Parameter isAnonymous: Set this to true if the user is anonymous,
+     for example when joining a meeting with a share link.


Putting this in separate lines make them harder to read

sacheu · 2021-02-24T23:46:28Z

In general, in some places, breakin the text into 2 lines make the comment harder to read.

JoshuaLai · 2021-02-24T23:51:03Z

In general, in some places, breakin the text into 2 lines make the comment harder to read.

Yes I did that on purpose to fix warnings. There was a bunch of warnings the linter was complaining about.

sacheu · 2021-02-25T01:12:56Z

In general, in some places, breakin the text into 2 lines make the comment harder to read.

Yes I did that on purpose to fix warnings. There was a bunch of warnings the linter was complaining about.

I guess I don't understand why linter was complaining about. Since the line was not that long originally.

JoshuaLai · 2021-02-25T17:51:12Z

Just double checking, but yes the liner was complaining that the character limit is 120 and in some cases we were just a little bit over (124 characters) or very over (240 characters) so the goal was to just clean these up

sacheu · 2021-02-25T18:03:39Z

Is it possible to change the text to shorter so that they can be in same line? e.g.
"and" -> "&"
"Set this to true " -> "Set to true"
"for example" -> "e.g."

sacheu · 2021-02-25T18:05:57Z

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift


     - Parameter completionHandler: Closure that accepts an optional `AccessToken` or optional `Error` as parameters.
-     `AccessToken` returns  a token and an expiry date if applicable. `Error` returns `nil` if the current token can be returned.
+     `AccessToken` returns  a token and an expiry date if applicable.


there is an extra space between 'returns' and 'a'

sacheu · 2021-02-25T18:10:15Z

And in the Chat generated code, there is // swiftlint:disable line_length
I wonder if we should do this instead of breaking 1 sentence comment into 2 lines

https://github.com/Azure/azure-sdk-for-ios/blob/41039cc13f33b515d2fe353b478707df6840f140/sdk/communication/AzureCommunicationChat/Source/Generated/Models/UpdateChatMessageRequest.swift

sacheu · 2021-02-25T18:10:20Z

And in the Chat generated code, there is // swiftlint:disable line_length
I wonder if we should do this instead of breaking 1 sentence into 2 lines

https://github.com/Azure/azure-sdk-for-ios/blob/41039cc13f33b515d2fe353b478707df6840f140/sdk/communication/AzureCommunicationChat/Source/Generated/Models/UpdateChatMessageRequest.swift

JoshuaLai · 2021-02-26T00:29:11Z

.swiftlint.yml

@@ -1,2 +1,5 @@
 disabled_rules: # rule identifiers to exclude from running
  - large_tuple
+


@tjprescott @sacheu making a new addition to swiftlint line length rule to ignore comments.

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift

sacheu

Looks good to me. I think some changes can be reverted now.

sdk/communication/AzureCommunication/Source/Authentication/CommunicationTokenCredential.swift

sdk/communication/AzureCommunication/Source/Identifiers.swift

sdk/communication/AzureCommunication/Source/Authentication/CommunicationTokenCredential.swift

[Doc] Updated the communication docs and fixed warnings

0898487

JoshuaLai requested review from sacheu and a team as code owners February 23, 2021 19:59

ghost added the Communication label Feb 23, 2021

sacheu requested changes Feb 24, 2021

View reviewed changes

sacheu reviewed Feb 25, 2021

View reviewed changes

JoshuaLai added 2 commits February 25, 2021 15:20

[docs] remove double space

993a51f

[swiftlint] updating the linter to ignores_comments: true

5803891

JoshuaLai commented Feb 26, 2021

View reviewed changes

sacheu reviewed Feb 26, 2021

View reviewed changes

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift Outdated Show resolved Hide resolved

sacheu reviewed Feb 26, 2021

View reviewed changes

sdk/communication/AzureCommunication/Source/Authentication/AutoRefreshTokenCredential.swift Outdated Show resolved Hide resolved

sacheu approved these changes Feb 26, 2021

View reviewed changes

[Docs] revert the white space changes

68bdd68

sacheu reviewed Feb 26, 2021

View reviewed changes

sdk/communication/AzureCommunication/Source/Authentication/CommunicationTokenCredential.swift Outdated Show resolved Hide resolved

[Docs] revert from content white spaces changes

5a608c5

JoshuaLai merged commit d87f934 into Azure:master Feb 26, 2021

JoshuaLai deleted the communication/documentation branch February 26, 2021 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Communication] - Documentation - Update docs #730

[Communication] - Documentation - Update docs #730

JoshuaLai commented Feb 23, 2021

sacheu Feb 23, 2021

sacheu Feb 23, 2021

sacheu Feb 23, 2021

sacheu Feb 23, 2021

sacheu Feb 25, 2021

sacheu Feb 23, 2021

sacheu Feb 23, 2021

sacheu commented Feb 24, 2021

JoshuaLai commented Feb 24, 2021

sacheu commented Feb 25, 2021

JoshuaLai commented Feb 25, 2021

sacheu commented Feb 25, 2021 •

edited

Loading

sacheu Feb 25, 2021

sacheu commented Feb 25, 2021

sacheu commented Feb 25, 2021 •

edited

Loading

JoshuaLai Feb 26, 2021

sacheu left a comment

		@@ -1,2 +1,5 @@
		disabled_rules: # rule identifiers to exclude from running
		- large_tuple

[Communication] - Documentation - Update docs #730

[Communication] - Documentation - Update docs #730

Conversation

JoshuaLai commented Feb 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sacheu commented Feb 24, 2021

JoshuaLai commented Feb 24, 2021

sacheu commented Feb 25, 2021

JoshuaLai commented Feb 25, 2021

sacheu commented Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

sacheu commented Feb 25, 2021

sacheu commented Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

sacheu left a comment

Choose a reason for hiding this comment

sacheu commented Feb 25, 2021 •

edited

Loading

sacheu commented Feb 25, 2021 •

edited

Loading