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

Expose EffectType types enum #1099

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Conversation

challet
Copy link
Contributor

@challet challet commented Nov 13, 2024

From #1098 issue

  • expose the EffectType enum so it can be relied on to test effect objects and infer their members
if (effect.type_i == Horizon.ServerApi.EffectType.trustline_created) {
    return effect.limit; // which doesn't exist on all effects
}
  • Add a type_i to the Trade type, so it can also be tested and inferred. Including inferring it out from the previous example.

Note: I am not certain about from where it should be exported since it is a const from within a namespace only about types.

@challet challet marked this pull request as draft November 13, 2024 12:18
@challet challet marked this pull request as ready for review November 13, 2024 12:45
Copy link
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

Seems good! Nice work 👏 It'll be exported at the top level of the Horizon module, I take it?

@challet
Copy link
Contributor Author

challet commented Nov 13, 2024

Thanks ! Actually, no I've put it as Horizon.ServerApi.EffectType and that's what I wasn't sure about with

Note: I am not certain about from where it should be exported since it is a const from within a namespace only about types.

Should I move it to the top level then ?

@Shaptic
Copy link
Contributor

Shaptic commented Nov 13, 2024

Nope, in the Api is actually perfect 👍

@Shaptic Shaptic merged commit 3a4253a into stellar:master Nov 13, 2024
11 checks passed
BlaineHeffron pushed a commit to AhaLabs/js-stellar-sdk that referenced this pull request Nov 13, 2024
@challet challet mentioned this pull request Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants