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

Host example #116

Merged
merged 8 commits into from
Aug 31, 2018
Merged

Host example #116

merged 8 commits into from
Aug 31, 2018

Conversation

chfast
Copy link
Member

@chfast chfast commented Aug 30, 2018

Fixes #110.

@chfast chfast force-pushed the host-example branch 4 times, most recently from c7ddcf2 to b86f745 Compare August 30, 2018 13:27
// Declarations of functions from example host:

evmc_context* example_host_create_context();
void example_host_destroy_context(evmc_context* context);
Copy link
Member

Choose a reason for hiding this comment

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

Why not create a header for it now?

Copy link
Member Author

@chfast chfast Aug 30, 2018

Choose a reason for hiding this comment

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

I don't want it to be a part of the example.

};

/// Example how the API is supposed to be used.
int main()
Copy link
Member

Choose a reason for hiding this comment

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

I think this part should stay.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do: #118
I have idea how to more improve examples, but need to move some files. I'd prefer in following PRs.

Copy link
Member

Choose a reason for hiding this comment

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

Would it be hard to keep it in this PR? It should just use the context from example-host

Copy link
Member Author

Choose a reason for hiding this comment

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

This is done. What next?

@chfast chfast force-pushed the host-example branch 3 times, most recently from 4d7444b to d697018 Compare August 30, 2018 16:22
@chfast
Copy link
Member Author

chfast commented Aug 30, 2018

Done except it does not work now.

@chfast
Copy link
Member Author

chfast commented Aug 30, 2018

Needs #119

@axic
Copy link
Member

axic commented Aug 31, 2018

This may be nitpicking, but I'd like to have git mv capi.c example-host.c in Replace capi.c exampled with example_host.cpp, so to avoid the need for the second commit and keep the tiny history of it.

@chfast
Copy link
Member Author

chfast commented Aug 31, 2018

Good now?

@axic
Copy link
Member

axic commented Aug 31, 2018

I meant git mv capi.c examples.c and then removing everything except main (in the same commit), that should keep the history.

I can do it on a different branch if you want to have a look at it?

@chfast
Copy link
Member Author

chfast commented Aug 31, 2018

No, I will do it.

@chfast
Copy link
Member Author

chfast commented Aug 31, 2018

Actually I did it in ab480f2. It will not work because changes are too big.

@axic
Copy link
Member

axic commented Aug 31, 2018

Tried myself, it seems even if it is a git move, it thinks the diff is too large and just keeps it as delete/create.

@chfast
Copy link
Member Author

chfast commented Aug 31, 2018

I can add intermediate commit to have it.


evmc_uint256be balance(evmc_context* context, const evmc_address* address)
static evmc_uint256be balance(evmc_context* context, const evmc_address* address)
Copy link
Member

Choose a reason for hiding this comment

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

Not relevant to this PR, but I guess if this is .cpp we should use an anonymous namespace instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm trying to stay in C dialect as much as possible.

@axic
Copy link
Member

axic commented Aug 31, 2018

I can add intermediate commit to have it.

Would be nice in order to see the actual changes in main, but I'm not pushing full shoulder for it.

@chfast
Copy link
Member Author

chfast commented Aug 31, 2018

I have it, at least locally: 98213f9#diff-ca5036d6b23291fb8dcac8244c1bc9a6

For some reason GitHub does not show the history of example.c, but my git does.

@axic
Copy link
Member

axic commented Aug 31, 2018

It does have the move now. Github doesn't have --follow enable on git log infortunately, so in the history of the individual file it won't show up, but works locally :)

@chfast chfast merged commit 3a3aef2 into master Aug 31, 2018
@chfast chfast deleted the host-example branch August 31, 2018 11:34
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.

2 participants