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

refactor(id_type): use macros for defining ID types and implementing common traits #5471

Merged
merged 10 commits into from
Jul 30, 2024

Conversation

SanchithHegde
Copy link
Member

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

This PR introduces declarative macros to define ID types and implement common traits such as Debug, FromSql, ToSql, etc. on these ID types. In addition, this PR replaces the usages of the from() associated function (to construct the ID types) with the TryFrom trait implementation.

Motivation and Context

Helps reduce duplication of boilerplate code.

How did you test it?

Sanity testing using Postman collection, code should work if it compiles.

Checklist

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

@SanchithHegde SanchithHegde added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor api-v2 labels Jul 29, 2024
@SanchithHegde SanchithHegde added this to the July 2024 Release milestone Jul 29, 2024
@SanchithHegde SanchithHegde self-assigned this Jul 29, 2024
@SanchithHegde SanchithHegde requested review from a team as code owners July 29, 2024 11:22
Narayanbhat166
Narayanbhat166 previously approved these changes Jul 30, 2024
Copy link
Member

@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

Narayanbhat166
Narayanbhat166 previously approved these changes Jul 30, 2024
srujanchikke
srujanchikke previously approved these changes Jul 30, 2024
Copy link
Contributor

@srujanchikke srujanchikke left a comment

Choose a reason for hiding this comment

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

reviewed connector part and LGTM.

Aprabhat19
Aprabhat19 previously approved these changes Jul 30, 2024
ThisIsMani
ThisIsMani previously approved these changes Jul 30, 2024
Copy link
Contributor

@ThisIsMani ThisIsMani left a comment

Choose a reason for hiding this comment

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

Dashboard specific changes looks fine.

lsampras
lsampras previously approved these changes Jul 30, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 30, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 30, 2024
@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 30, 2024
@SanchithHegde SanchithHegde removed this pull request from the merge queue due to a manual request Jul 30, 2024
jarnura
jarnura previously approved these changes Jul 30, 2024
@likhinbopanna likhinbopanna enabled auto-merge July 30, 2024 13:44
@likhinbopanna likhinbopanna added this pull request to the merge queue Jul 30, 2024
Merged via the queue into main with commit 1d4fb1d Jul 30, 2024
14 checks passed
@likhinbopanna likhinbopanna deleted the id-type-macros branch July 30, 2024 15:08
@SanchithHegde SanchithHegde removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-v2 C-refactor Category: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants