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.

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

refactor: Make the metadata dynamic via implementing JsonString #4732

Merged
merged 2 commits into from
Jun 27, 2024
Merged

refactor: Make the metadata dynamic via implementing JsonString #4732

merged 2 commits into from
Jun 27, 2024

Conversation

Stukalov-A-M
Copy link
Contributor

Description

  • Removed MetadataValueBox
  • Added JsonString

Linked issue

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch from 737bc02 to 3841f56 Compare June 14, 2024 10:12
@github-actions github-actions bot added the api-changes Changes in the API for client libraries label Jun 14, 2024
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch 2 times, most recently from b130a1b to 0366522 Compare June 14, 2024 10:50
mversic
mversic requested changes Jun 14, 2024
View reviewed changes
mversic
mversic reviewed Jun 14, 2024
View reviewed changes
mversic
mversic reviewed Jun 14, 2024
View reviewed changes
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch from 0366522 to 58064de Compare June 14, 2024 11:53
@nxsaken
Copy link
Contributor

nxsaken commented Jun 14, 2024

Please use the imperative mood in commit messages / PR titles to keep it consistent

@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch 3 times, most recently from 65fb73a to 6bd36fd Compare June 16, 2024 19:49
@Stukalov-A-M Stukalov-A-M changed the title refactor: Metadata became dynamic and consists of JsonString refactor: Make the metadata dynamic via implementing JsonString Jun 16, 2024
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch from 6bd36fd to 84ecc99 Compare June 16, 2024 19:55
@Stukalov-A-M Stukalov-A-M requested a review from mversic June 16, 2024 19:56
Stukalov-A-M
Stukalov-A-M commented Jun 17, 2024
View reviewed changes
mversic
mversic requested changes Jun 19, 2024
View reviewed changes
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch from f3c33bf to d3f8dd6 Compare June 19, 2024 11:30
@Stukalov-A-M Stukalov-A-M requested review from mversic and Erigara June 19, 2024 11:30
@Stukalov-A-M Stukalov-A-M mentioned this pull request Jun 20, 2024
Closed
@Stukalov-A-M
Copy link
Contributor Author

Created a a task for unit tests creation
#4748

@Stukalov-A-M Stukalov-A-M mentioned this pull request Jun 20, 2024
Closed
Erigara
Erigara reviewed Jun 26, 2024
View reviewed changes
mversic
mversic requested changes Jun 26, 2024
View reviewed changes
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch 2 times, most recently from 710d238 to 1ed9e5b Compare June 27, 2024 07:45
@Stukalov-A-M Stukalov-A-M force-pushed the dynamic-metadata-values branch from 1ed9e5b to 399fd81 Compare June 27, 2024 08:33
@mversic
Copy link
Contributor

mversic commented Jun 27, 2024

@Stukalov-A-M IMO you can add #[ignore to the failing test and fix it in a separate PR

@Stukalov-A-M Stukalov-A-M merged commit db96164 into hyperledger-iroha:main Jun 27, 2024
9 of 11 checks passed
@Stukalov-A-M Stukalov-A-M deleted the dynamic-metadata-values branch June 27, 2024 09:00
@nxsaken
Copy link
Contributor

nxsaken commented Jun 28, 2024

@Stukalov-A-M the integration test correct_pagination_assets_after_creating_new_one has been failing since merging this. Can you fix or ignore the test as suggested above?

@Stukalov-A-M
Copy link
Contributor Author

@Stukalov-A-M the integration test correct_pagination_assets_after_creating_new_one has been failing since merging this. Can you fix or ignore the test as suggested above?

I don't think it's possible in the closed PR. You may add ignore if u have an opened PR.

hollermay pushed a commit to hollermay/iroha that referenced this pull request Jun 28, 2024
@Stukalov-A-M @hollermay
refactor: Make the metadata dynamic via implementing JsonString (hype…
5e2c420 
…rledger-iroha#4732)

Signed-off-by: Stukalov-A-M <[email protected]>
@nxsaken
Copy link
Contributor

nxsaken commented Jul 1, 2024

@Stukalov-A-M I meant in a separate PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Erigara Erigara Erigara approved these changes

@mversic mversic mversic approved these changes

@s8sato s8sato Awaiting requested review from s8sato s8sato is a code owner

@SamHSmith SamHSmith Awaiting requested review from SamHSmith

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@0x009922 0x009922 Awaiting requested review from 0x009922 0x009922 is a code owner

@DCNick3 DCNick3 Awaiting requested review from DCNick3 DCNick3 is a code owner

@VAmuzing VAmuzing Awaiting requested review from VAmuzing

@dima74 dima74 Awaiting requested review from dima74 dima74 is a code owner

Assignees

@Erigara Erigara

@mversic mversic

@Stukalov-A-M Stukalov-A-M

Labels
api-changes Changes in the API for client libraries
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dynamic metadata value
4 participants
@Stukalov-A-M @nxsaken @mversic @Erigara