Skip to content

feat: allow (de)serializing countries to/from alpha-2, alpha-3 and numeric country codes #836

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.

Sign up for GitHub

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

Jump to bottom
Merged
jarnura merged 19 commits into from
Apr 6, 2023
Merged

feat: allow (de)serializing countries to/from alpha-2, alpha-3 and numeric country codes #836

jarnura merged 19 commits into from
Apr 6, 2023

Conversation

@nachiketkanore
Copy link
Contributor

@nachiketkanore nachiketkanore commented Apr 4, 2023

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

Description

fixes #804

Additional Changes

  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

Added multiple test cases covering maximum scenarios
Tested with cargo test with all OK passed

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code
  • I added unit tests for my changes where possible
  • I added a CHANGELOG entry if applicable

@nachiketkanore nachiketkanore requested review from a team, ashokkjag and jarnura as code owners April 4, 2023 09:12
Narayanbhat166
Narayanbhat166 previously approved these changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@Narayanbhat166 Narayanbhat166 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Awesome work!

@nachiketkanore
Copy link
Contributor Author

nachiketkanore commented Apr 4, 2023

Any specific reason these tests are failing?
My changes seem unrelated to them (except formatting)

SanchithHegde
SanchithHegde reviewed Apr 4, 2023
View reviewed changes
Copy link
Member

@SanchithHegde SanchithHegde left a comment

Choose a reason for hiding this comment

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

Other than that, looks good to me! Thanks for your contribution by the way!

@SanchithHegde SanchithHegde added C-feature Category: Feature request or enhancement S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Apr 4, 2023
@SanchithHegde SanchithHegde changed the title Country codes feat: allow (de)serializing countries to/from alpha-2, alpha-3 and numeric country codes Apr 4, 2023
@nachiketkanore
Copy link
Contributor Author

nachiketkanore commented Apr 5, 2023

Suggested changes done

SanchithHegde
SanchithHegde reviewed Apr 5, 2023
View reviewed changes
Copy link
Member

@SanchithHegde SanchithHegde left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Other than that, looks good to me!

Edit: Also, please address the formatting check and clippy lints.

Narayanbhat166
Narayanbhat166 previously approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@Narayanbhat166 Narayanbhat166 left a comment

Choose a reason for hiding this comment

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

Looks Good to me!

@SanchithHegde SanchithHegde added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Apr 6, 2023
@jarnura jarnura removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Apr 6, 2023
@jarnura jarnura added this pull request to the merge queue Apr 6, 2023
Merged via the queue into juspay:main with commit 899767c Apr 6, 2023
@Gnanasundari24 Gnanasundari24 mentioned this pull request Apr 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jarnura jarnura jarnura approved these changes

@SanchithHegde SanchithHegde SanchithHegde approved these changes

@ashokkjag ashokkjag Awaiting requested review from ashokkjag

+1 more reviewer

@Narayanbhat166 Narayanbhat166 Narayanbhat166 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@nachiketkanore nachiketkanore

Labels

C-feature Category: Feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEATURE]: add support for three letter and numeric country codes

4 participants

@nachiketkanore @jarnura @SanchithHegde @Narayanbhat166