Skip to content

feature: Issue #17 - Registration Number Policies#40

Merged
PooriaT merged 10 commits intofeature/create-api-client-policy-modulefrom
feature/policy/mock_policies_for_registration_number_#17
Feb 23, 2024
Merged

feature: Issue #17 - Registration Number Policies#40
PooriaT merged 10 commits intofeature/create-api-client-policy-modulefrom
feature/policy/mock_policies_for_registration_number_#17

Conversation

@PooriaT
Copy link

@PooriaT PooriaT commented Feb 14, 2024

Description

Issue Link: Mock policies for registration number #17

In this issue, a verification for the registration number is provided. First, the registration number has to be validated. Then, it should be checked whether it is available in Vancouver Open Data.

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

  • Unit Test
  • Integration Test
  • E2E Test

Checklist:

Before you submit your pull request, please make sure you have completed the following:

  • I have read the CONTRIBUTING document.
  • I have checked that my code adheres to the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • Any dependent changes have been merged and published in downstream modules.

@PooriaT PooriaT requested a review from umsu2 as a code owner February 14, 2024 05:32
@PooriaT PooriaT requested a review from nam-m February 14, 2024 05:32
Copy link
Contributor

@nam-m nam-m left a comment

Choose a reason for hiding this comment

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

I think the policy structure is good, and it's very readable.
You don't need to call the API in here though, because it is handled in #38

@nam-m nam-m mentioned this pull request Feb 22, 2024
16 tasks
@PooriaT PooriaT changed the base branch from main to feature/create-api-client-policy-module February 22, 2024 23:19
@PooriaT PooriaT requested a review from nam-m February 22, 2024 23:19
Copy link
Contributor

@nam-m nam-m left a comment

Choose a reason for hiding this comment

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

Thanks for your update

2 changes I recommend are:

  1. Remove total_count check in the policies
  2. In the tests, you can set registration numbers one time outside(maybe parametrize too), and let all tests go through these

@PooriaT PooriaT merged commit 5d78234 into feature/create-api-client-policy-module Feb 23, 2024
@PooriaT PooriaT deleted the feature/policy/mock_policies_for_registration_number_#17 branch February 23, 2024 16:37
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