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

Implemented data transformation logic #2547

Closed
wants to merge 1 commit into from
Closed

Implemented data transformation logic #2547

wants to merge 1 commit into from

Conversation

integraledelebesgue
Copy link
Member

@integraledelebesgue integraledelebesgue commented Oct 1, 2024 •
edited
Loading

This was referenced Oct 1, 2024
Closed
Closed
Closed
Draggu
Draggu requested changes Oct 2, 2024
View reviewed changes
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/sierra_abi.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/sierra_abi.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
cptartur
cptartur reviewed Oct 2, 2024
View reviewed changes
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/calldata_representation.rs Outdated Show resolved Hide resolved
crates/sncast/src/helpers/data_transformer/transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
crates/sncast/tests/integration/data_transformer.rs Outdated Show resolved Hide resolved
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 2 times, most recently from 6a0a9d2 to dd60ddb Compare October 2, 2024 14:02
@integraledelebesgue integraledelebesgue force-pushed the spr/master/13b07d29 branch 2 times, most recently from c5d5e25 to 26f846e Compare October 3, 2024 09:14
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 3 times, most recently from 7c6406d to 4331279 Compare October 3, 2024 09:52
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 4 times, most recently from f2e1ad8 to 0791178 Compare October 3, 2024 14:58
@integraledelebesgue integraledelebesgue mentioned this pull request Oct 4, 2024
Merged
@integraledelebesgue integraledelebesgue force-pushed the spr/master/13b07d29 branch 2 times, most recently from 0f0ee96 to dcd604e Compare October 4, 2024 19:08
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 2 times, most recently from 3f7e33b to c672d3a Compare October 8, 2024 18:16
@piotmag769
Copy link
Member

Wouldn't it be better to be a separate crate? It could be reused in different places then

@integraledelebesgue
Copy link
Member Author

Wouldn't it be better to be a separate crate? It could be reused in different places then

That sounds like a good idea. cc: @cptartur

@integraledelebesgue integraledelebesgue changed the title Introduced data transformer Implemented data transformation logic Oct 11, 2024
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 2 times, most recently from d25b4a5 to da7c243 Compare October 11, 2024 12:04
cptartur
cptartur approved these changes Oct 15, 2024
View reviewed changes
crates/data-transformer/src/sierra_abi/complex_types.rs Show resolved Hide resolved
crates/data-transformer/src/sierra_abi/data_representation.rs
{
value
.parse::<T>()
.context(neat_parsing_error_message(value, type_name::<T>(), None))
Copy link
Member

Choose a reason for hiding this comment

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

Is using type_name safe? https://doc.rust-lang.org/std/any/fn.type_name.html

Copy link
Contributor

Choose a reason for hiding this comment

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

its not

crates/data-transformer/src/sierra_abi/macros.rs Show resolved Hide resolved
crates/data-transformer/src/sierra_abi/mod.rs Outdated Show resolved Hide resolved
crates/data-transformer/src/transformer.rs Outdated Show resolved Hide resolved
@integraledelebesgue integraledelebesgue force-pushed the spr/master/8d990066 branch 2 times, most recently from 9d004a6 to 6c5b6c8 Compare October 16, 2024 13:05
@integraledelebesgue
Copy link
Member Author

✓ Commit merged in pull request #2555

github-merge-queue bot pushed a commit that referenced this pull request Oct 16, 2024
@integraledelebesgue
Introduced Data Transformer (#2555)
a2e3c7d 
**Stack**:
- #2555
- #2548
- #2547
- #2546
- #2545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Draggu Draggu Draggu approved these changes

@cptartur cptartur cptartur approved these changes

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

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

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@integraledelebesgue @piotmag769 @cptartur @Draggu