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

Pia 4595 extract feedback from cleanup #360

Merged
merged 6 commits into from
Oct 31, 2023

Conversation

abolfazlimahdi
Copy link
Contributor

No description provided.

@abolfazlimahdi abolfazlimahdi self-assigned this Oct 16, 2023
Copy link
Contributor

@a-szotyori a-szotyori left a comment

Choose a reason for hiding this comment

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

Sorry for the many requests. Most of them are about the documentation.

Please also update the extraction feedback integration tests:

  • Rename to TransferSummaryIntegrationTest
  • Replace deprecated cleanup() with sendTransferSummary() and cleanup().

The integration tests can be found here:

  • capture-sdk/default-network/src/androidTest/java/net/gini/android/capture/network/ExtractionFeedbackIntegrationTest.kt
  • bank-sdk/sdk/src/androidTest/java/net/gini/android/bank/sdk/ExtractionFeedbackIntegrationTest.kt

Thank you!

@abolfazlimahdi abolfazlimahdi force-pushed the PIA-4595-extract-feedback-from-cleanup branch 2 times, most recently from 3890d11 to b6826e6 Compare October 17, 2023 12:58
Copy link
Contributor

@a-szotyori a-szotyori left a comment

Choose a reason for hiding this comment

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

Nice! 👏

/**
* Provides transfer summary to Gini.
*
* Please provide the required transfer summary to improve the future extraction accuracy.
Copy link
Contributor

Choose a reason for hiding this comment

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

[Suggestion] we have 4 sentences in row, which are starting from Please. I propose to keep it till Please follow the recommendations below and remove other please after.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zladzeyka Just to be on the same page, you mean I delete the pleases after the first one?

- rename ExtractionFeedbackIntegrationTest to TransferSummaryIntegrationTest
- rename feedback to transfer summary in comments and menu items
- add some information about how to use sendTransferSummary method
- enhance Kotlin and Java docs

PIA-4595
@abolfazlimahdi abolfazlimahdi force-pushed the PIA-4595-extract-feedback-from-cleanup branch from b6826e6 to a32ba46 Compare October 30, 2023 16:55
@abolfazlimahdi abolfazlimahdi merged commit b7fd984 into main Oct 31, 2023
47 checks passed
@abolfazlimahdi abolfazlimahdi deleted the PIA-4595-extract-feedback-from-cleanup branch October 31, 2023 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants