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: move schema ids to constants #550

Merged
merged 2 commits into from
Jul 12, 2023
Merged

refactor: move schema ids to constants #550

merged 2 commits into from
Jul 12, 2023

Conversation

iamacook
Copy link
Member

This prevents typos of schema $ids by moving them to constants and importing them in their relevant validators.

@iamacook iamacook self-assigned this Jul 11, 2023
@iamacook iamacook requested a review from a team as a code owner July 11, 2023 09:13
@coveralls
Copy link

coveralls commented Jul 11, 2023

Pull Request Test Coverage Report for Build 5529319081

  • 117 of 117 (100.0%) changed or added relevant lines in 77 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 92.383%

Totals Coverage Status
Change from base Build 5518050421: 0.2%
Covered Lines: 4313
Relevant Lines: 4565

💛 - Coveralls

@hectorgomezv hectorgomezv added the in review Someone is reviewing this Pull Request label Jul 12, 2023
Copy link
Member

@hectorgomezv hectorgomezv left a comment

Choose a reason for hiding this comment

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

Great job 👏🏻👏🏻

I've left a nitpick comment only 🙂

export const dataDecodedParameterSchema: Schema = {
$id: 'https://safe-client.safe.global/schemas/data-decoded/data-decoded-parameter.json',
$id: DATA_DECODED_PARAMTER_SCHEMA_ID,
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a typo here: DATA_DECODED_PARAMTER_SCHEMA_ID, it should be DATA_DECODED_PARAMETER_SCHEMA_ID 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch! I've fixed it in cb1f6e2.

@hectorgomezv hectorgomezv removed the in review Someone is reviewing this Pull Request label Jul 12, 2023
@hectorgomezv hectorgomezv self-requested a review July 12, 2023 09:32
@iamacook iamacook merged commit ce71beb into main Jul 12, 2023
16 checks passed
@iamacook iamacook deleted the schema-id-constants branch July 12, 2023 09:35
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