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

prefix spec resource with version id #837

Merged

Conversation

nagmo-starkware
Copy link
Contributor

@nagmo-starkware nagmo-starkware commented Jul 12, 2023

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

  • Yes
  • No

Other information


This change is Reviewable

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #837 (762819c) into main (fa6d9ea) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #837   +/-   ##
=======================================
  Coverage   72.03%   72.03%           
=======================================
  Files          46       46           
  Lines        4399     4399           
  Branches     4399     4399           
=======================================
  Hits         3169     3169           
  Misses        569      569           
  Partials      661      661           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 739fc25 to 56e2dc8 Compare July 12, 2023 07:33
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 7470573 to 0061a85 Compare July 12, 2023 07:33
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 56e2dc8 to 71885c4 Compare July 12, 2023 08:02
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 0061a85 to ed2cd48 Compare July 12, 2023 08:02
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 71885c4 to c6a0a30 Compare July 12, 2023 08:28
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from ed2cd48 to a041f4f Compare July 12, 2023 08:28
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from c6a0a30 to 5c5c9b4 Compare July 12, 2023 09:02
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from a041f4f to ae13bcd Compare July 12, 2023 09:02
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 5c5c9b4 to de47558 Compare July 12, 2023 09:59
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from ae13bcd to 0be6f97 Compare July 12, 2023 09:59
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from de47558 to 6a9ef36 Compare July 12, 2023 11:38
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 0be6f97 to d66aa33 Compare July 12, 2023 11:38
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 6a9ef36 to 7b77bb7 Compare July 12, 2023 11:45
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from d66aa33 to 302244b Compare July 12, 2023 11:45
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-api branch from 7b77bb7 to 867a302 Compare July 12, 2023 11:50
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 302244b to d44ba99 Compare July 12, 2023 11:50
Base automatically changed from refactor-gateway-for-multiversion-implementation-api to main July 12, 2023 12:15
@dan-starkware
Copy link
Collaborator

crates/papyrus_gateway/src/test_utils.rs line 39 at r1 (raw file):

    version_id: &str,
) -> JSONSchema {
    let target = format!("./resources/{}_starknet_api_openrpc.json", version_id);

Can you use the following?

Suggestion:

format!("./resources/{version_id}_starknet_api_openrpc.json");

dan-starkware
dan-starkware previously approved these changes Jul 12, 2023
Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @DvirYo-starkware and @nagmo-starkware)

Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @DvirYo-starkware and @nagmo-starkware)


crates/papyrus_gateway/src/v0_3_0/api/test.rs line 42 at r3 (raw file):

    JsonRpcError, Tag,
};
use crate::test_utils::{

Is this related to the PR?

Copy link
Contributor Author

@nagmo-starkware nagmo-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @dan-starkware and @DvirYo-starkware)


crates/papyrus_gateway/src/v0_3_0/api/test.rs line 42 at r3 (raw file):

Previously, dan-starkware wrote…

Is this related to the PR?

yes I'm sending the version name to put it in the shcema path

Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @DvirYo-starkware)

dan-starkware
dan-starkware previously approved these changes Jul 18, 2023
Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @DvirYo-starkware)

@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 10df8a0 to 51c17d7 Compare July 19, 2023 12:42
@nagmo-starkware nagmo-starkware mentioned this pull request Jul 19, 2023
9 tasks
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 51c17d7 to 4c4f85d Compare July 20, 2023 10:23
@nagmo-starkware nagmo-starkware force-pushed the refactor-gateway-for-multiversion-implementation-spec branch from 4c4f85d to 762819c Compare July 20, 2023 13:04
Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @DvirYo-starkware)

Copy link
Collaborator

@dan-starkware dan-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @DvirYo-starkware)

@dan-starkware dan-starkware added this pull request to the merge queue Jul 21, 2023
Merged via the queue into main with commit dba444f Jul 21, 2023
15 checks passed
@dan-starkware dan-starkware deleted the refactor-gateway-for-multiversion-implementation-spec branch July 21, 2023 06:39
@github-actions github-actions bot locked and limited conversation to collaborators Jul 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants