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

Add cashback api #52

Merged
merged 11 commits into from
Jan 24, 2021
Merged

Add cashback api #52

merged 11 commits into from
Jan 24, 2021

Conversation

daikidomon
Copy link

@daikidomon daikidomon commented Jan 4, 2021

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you read and signed the automated Contributor's License Agreement?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you lint your code locally prior to submission?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@CLAassistant
Copy link

CLAassistant commented Jan 4, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ shreyanshp
❌ Daiki Domon


Daiki Domon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@daikidomon
Copy link
Author

@shreyanshp

Just curious to know what does this variable do and why do we need this?

Added to make the format consistent.

@daikidomon
Copy link
Author

@shreyanshp

I have some questions.

  • Do I need to create a new test code for cashback?
  • If I create these codes, what userAuthorizationId do I use?

@blackduck-copilot
Copy link

Black Duck Security Report

Merging #52 into master will not change security risk.

Click here to see full report

@shreyanshp
Copy link
Contributor

@daikidomon Thanks for the PR, you can use mocks, in the same way the existing tests works, and use xxxx or any dummy value for tokens and user authorizations

@shreyanshp
Copy link
Contributor

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Daiki Domon seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

#52 (comment)

@daikidomon Thanks for the PR, I can see that the commits are not linked to a github account

@daikidomon
Copy link
Author

@shreyanshp
Added unit test, please review.

@sonarcloud
Copy link

sonarcloud bot commented Jan 21, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@shreyanshp
Copy link
Contributor

@daikidomon Looks good!
Could you please add the docstring for the functions that would be helpful

https://app.codacy.com/gh/paypay/paypayopa-sdk-python/pullRequest?bid=21658699&prid=6748025

@shreyanshp shreyanshp merged commit 18727fc into paypay:master Jan 24, 2021
@shreyanshp
Copy link
Contributor

@daikidomon Thank you so much for the PR! Glad to have you here!

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.

None yet

3 participants