Skip to content
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

[TW-400] Update comments for ForageSDK #153

Merged
merged 7 commits into from
Jan 16, 2024

Conversation

kimberleehowley
Copy link
Contributor

What

Updates comments for the ForageSDK.kt file.

Why

Better comments help future-proof the docs.

Test Plan

  • ❌ I don't need to add unit tests for this, since I've only changed comments.
  • ✅ | The reviewer should manually test the changes in this PR. To test the changes, check out this branch, and then run:
./gradlew dokkaHtml
npx http-serve reference-docs 

How

There are no extra steps to the rollout beyond merging this PR, since these changes are code comments!

@kimberleehowley kimberleehowley added the documentation Improvements or additions to documentation label Jan 9, 2024
@kimberleehowley kimberleehowley self-assigned this Jan 9, 2024
Copy link
Contributor

@djoksimo djoksimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @kimberleehowley !!

My main change requests is regarding ForageSDK and the bit about Forage Payment objects; most of my other comments are suggestions and can be deferred to future tickets!

Also, it looks like you need to run the linting command to fix the linting issues in forage-android-sdk

./gradlew spotlessApply

Comment on lines 30 to 33
* * [capturePayment]
* * [checkBalance]
* * [deferPaymentCapture]
* * [tokenizeEBTCard]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we put this in the common order (tokenize -> checkBalance -> deferPaymentCapture -> capturePayment) these and hyperlink them with descriptions?

Suggested change
* * [capturePayment]
* * [checkBalance]
* * [deferPaymentCapture]
* * [tokenizeEBTCard]
* * [Tokenizing card information][tokenizeCard]
* * [Checking the balance of a card][checkBalance]
* * [Collecting a customer's PIN for a payment and deferring the capture of the payment to the server][deferPaymentCapture]
* * [Immediately capturing a payment][capturePayment]

@@ -83,17 +93,13 @@ class ForageSDK : ForageSDKInterface {
}

/**
* Checks the balance of a given PaymentMethod using a ForagePINEditText
* Checks the balance of a previously created [`PaymentMethod`](https://docs.joinforage.app/reference/payment-methods) via a [ForagePINEditText][com.joinforage.forage.android.ui.ForagePINEditText] Element.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To address the issue involving the lack of newlines. What are your thoughts on the following list item-based workaround for the submit methods in this file?

Also:

  • can we wrap long lines to fit roughly inside the right-hand vertical line in Android studio (around 100 character line length limit)
    • the comments should be digestible for clients reading the comments inside Android Studio, and not just the docs site.
  • can we use the notation @param <parameter-name-not-type-here> Description with reference to type of parameter here (as seen in the example below). Sticking with the KDoc standard should reduce the risk of our Dokka breaking and should provide a more familiar + consistent format throughout our comments.
    /**
     * Checks the balance of a previously created [`PaymentMethod`](https://docs.joinforage.app/reference/payment-methods)
     * via a [ForagePINEditText][com.joinforage.forage.android.ui.ForagePINEditText] Element.
     *
     * * On success, the response includes `snap` and `cash` fields that indicate the EBT Card's current SNAP and EBT Cash balances.
     * * On failure, for example in the case of [`ebt_error_14`](https://docs.joinforage.app/reference/errors#ebt_error_14), 
     * the response includes a list of [ForageError][com.joinforage.forage.android.network.model.ForageError] objects that you can unpack to troubleshoot the issue.
     *
     * @param params A [CheckBalanceParams][com.joinforage.forage.android.network.model] model
     * that passes a [`foragePinEditText`][com.joinforage.forage.android.ui.ForagePINEditText] instance
     * and a `paymentMethodRef`, found in the response from a call to [tokenizeEBTCard] or the [Create a `PaymentMethod`](https://docs.joinforage.app/reference/create-payment-method) endpoint.
     * @throws [ForageConfigNotSetException] If the [ForageConfig] is not set for the provided `foragePinEditText`.
     * @see * [SDK errors](https://docs.joinforage.app/reference/errors#sdk-errors) for more information on error handling.
     * * [Test EBT Cards](https://docs.joinforage.app/docs/test-ebt-cards#balance-inquiry-exceptions) to trigger balance inquiry exceptions during testing.
     * @return A [ForageApiResponse] object.
     */
image

@kimberleehowley kimberleehowley merged commit 79dc8fd into main Jan 16, 2024
4 checks passed
@kimberleehowley kimberleehowley deleted the kimberlee/tw-400-update-comments-for-foragesdk branch January 16, 2024 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants