Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the project documentation to introduce the Cloud Device API and deprecate older Terminal API integrations. Key changes include a reorganized README, updated prerequisites, and the addition of dedicated documentation files for the Cloud Device and Terminal APIs. The review feedback identifies several issues within the new documentation's code snippets, such as incorrect package imports, mismatches between documented method names and their actual implementations, and the use of uninitialized variables in examples.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the CloudDeviceApi as a modern, OpenAPI-generated replacement for the legacy TerminalCloudAPI for in-person payments. The changes include updated documentation in the README.md, a new migration guide, and dedicated documentation files for both the new and legacy APIs. Feedback focuses on correcting a typo in the migration guide, resolving an inconsistency in example links within the README.md, and including missing standard library imports in the code snippets provided in the new documentation.
gcatanese
force-pushed
the
cloud-device-api-docs
branch
from
6f94229 to
67b4176
Compare
gcatanese
force-pushed
the
cloud-device-api-docs
branch
from
67b4176 to
3f71855
Compare
| .adyenCryptoVersion(0) | ||
| .keyIdentifier("CryptoKeyIdentifier12345") | ||
| .keyVersion(0) | ||
| .passphrase("p@ssw0rd123456"); |
There was a problem hiding this comment.
Should we suggest here loading these via an environment variable? some merchants may think this means it's ok to hardcode the passphrase
There was a problem hiding this comment.
Fair point, but all README files in the libraries use the same convention. I suggest to create a new issue if we want to include best practices in the snippets across all libraries.
|
|
||
| #### Numeric fields | ||
|
|
||
| - `BigInteger` fields are `Integer` in the Cloud device API |
There was a problem hiding this comment.
Out of curiosity, is there a risk this creates issues? this is a narrowing conversion
There was a problem hiding this comment.
Yes, there is always a risk "narrowing" the conversion, but practically Integer is big enough for the type of data we handle. It is most likely that former SDK code didn't really need to use BigInteger
gcatanese
force-pushed
the
cloud-device-api-docs
branch
from
3caa65f to
f7d71bd
Compare
gcatanese
force-pushed
the
cloud-device-api-docs
branch
from
f7d71bd to
967585f
Compare
2 participants
Update README to add new Cloud Device API
Created
docfolder to separateTerminal APIandCloud device APIdocAdded a section
Helper classesin theREADME(not the helper doesn't yet exist, it will included in a different PR)Added Migration Guide to be published with the Release
Note: Terminal API is not marked as deprecated, at this stage the approach is to promote the new Cloud device API for new integrations, making sure customers know that the existing integrations are not affected