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

update transaction status::accepted #46

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

alastairong1
Copy link
Contributor

Updates API for new transactionStatus struct

Copy link
Member

@zo-el zo-el left a comment

Choose a reason for hiding this comment

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

Can we update this common crate here.

So that we have the types in one place

@alastairong1
Copy link
Contributor Author

So why do these types exist in the hpos-api code if they're imported from the common crate?

@peeech
Copy link
Contributor

peeech commented Aug 22, 2024

Those are holofuel - specific types I think and because holofuel does not expose public crate with those types we are just manually defining them in the code of both hpos-api-rust and hpos_hc_connect. Shall we define them in one and import to another? I don't think so because they are holofuel specific and the only proper solution would be to make them available via holofuel types crate.

@alastairong1
Copy link
Contributor Author

@peeech @zo-el Ok so two different opinions...what do we do?

@zo-el
Copy link
Member

zo-el commented Aug 22, 2024

Shall we define them in one and import to another? I don't think so because they are holofuel specific and the only proper solution would be to make them available via holofuel types crate.

I agree with this in general, but its a lot of work to extract it from the transactor_integrity zome. (I wish it was easier, zippy has tried it too) Thats why we created it in hpos_hc_connect for now.

here was may attempt to extract it fro the transactor_integrity zome, but its not completed

@alastairong1
Copy link
Contributor Author

Ok I'm just going to update hops-connect

Copy link
Member

@zo-el zo-el left a comment

Choose a reason for hiding this comment

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

accepting to speed up the bug fix

@alastairong1 alastairong1 merged commit 176670e into develop Aug 22, 2024
2 checks passed
@alastairong1 alastairong1 deleted the fix-tx-status-change branch August 22, 2024 15:35
zo-el pushed a commit that referenced this pull request Aug 22, 2024
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