update sample code #1
Conversation
| String clientId="__clientId__"; | ||
| String clientSecret="__clientSecret__"; | ||
| String projectId="__projectId__"; | ||
| String appId = "__appId__" |
There was a problem hiding this comment.
spaces around the = for all variables
| <dependencies> | ||
| <dependency> | ||
| <groupId>com.sinch.sdk</groupId> | ||
| <artifactId>java-sdk</artifactId> | ||
| <version>${sdk.version}</version> | ||
| <version>LATEST</version> |
There was a problem hiding this comment.
You will get warning while building with this version LATEST because it has been deprecated. Maven suggests to use specific version to have stability of the builds. Thats why we used property sdk.version
dependencies.dependency.version' for com.sinch.sdk:java-sdk:jar is either LATEST or RELEASE (both of them are being deprecated) @ line 22, column 14
This is also knows as bad practice as build becomes a "lottery" so maybe we should stay with sdk.version in the doc?
There was a problem hiding this comment.
if so it should alwasy be updated to the current version, or maybe have the wildcard 0.1.+ to get minor versions automatically?
There was a problem hiding this comment.
I would say this is a basic knowledge how to update versions of dependencies for someone who is using maven / gradle. So IMO what we had at the beginning was ok. But this is only my suggestion.
There was a problem hiding this comment.
I agree with Adam, if you think this really needs clarifying I would suggest a potential middle road being changing the 'variable' to be more descriptive, somehow pointing the user to the current version on maven that we display at the start of the readme.
| Messages messages = | ||
| Sinch.conversationApi(Region.US) // The region of the app | ||
| .messages(appId); | ||
| messages.send(new TextMessageRequest("Hi from the Sinch SDK!") |
There was a problem hiding this comment.
potentially add a newline here to clearly mark it as a separate step
No description provided.