Skip to content
This repository was archived by the owner on Apr 8, 2026. It is now read-only.

cpp: Rename result::raw() to result::release_raw()#293

Merged
chfast merged 2 commits intomasterfrom
cpp_result_release_raw
May 21, 2019
Merged

cpp: Rename result::raw() to result::release_raw()#293
chfast merged 2 commits intomasterfrom
cpp_result_release_raw

Conversation

@chfast
Copy link
Copy Markdown
Member

@chfast chfast commented May 15, 2019

No description provided.

@chfast chfast requested review from axic and gumb0 May 15, 2019 14:55
## [6.2.2] - unreleased

- Changed: [[#293](https://github.com/ethereum/evmc/pull/293)]
In C++ API `evmc::result::raw()` renamed to `evmc::result::release_raw()`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this breaking?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it is, but the comment should at least be merged into 6.2

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's breaking, but 6.2 has broken C++ API anyway. Besides, this method is used internally.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just checked, raw was introduced in 6.2.1 via #257. I don't think renaming it is fixing a bug, the bug was fixed before the renaming, wasn't it?

@chfast chfast force-pushed the cpp_result_release_raw branch from e2c87b5 to 3ffadec Compare May 16, 2019 06:56
@chfast chfast changed the base branch from release/6.2 to master May 16, 2019 06:57
@chfast
Copy link
Copy Markdown
Member Author

chfast commented May 16, 2019

Based on master.

Copy link
Copy Markdown
Contributor

@jakelang jakelang left a comment

Choose a reason for hiding this comment

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

leak_raw sounds ok as well. release_raw sounds to me like a destructor or something that releases the associated resources.

@axic
Copy link
Copy Markdown
Member

axic commented May 21, 2019

acquire_raw ?

@chfast chfast force-pushed the cpp_result_release_raw branch from 3ffadec to a70e297 Compare May 21, 2019 08:40
@chfast chfast changed the title cpp: Rename result::raw() to result::release_raw() cpp: Rename result::raw() to result::acquire_raw() May 21, 2019
@chfast
Copy link
Copy Markdown
Member Author

chfast commented May 21, 2019

Renamed to acquire_raw().

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented May 21, 2019

But "acquire" is the opposite of what happens. Maybe it could look fine if it were a free function like evmc_res = acquire_raw(res), but res.acquire_raw() look like result acquires something.

@chfast chfast force-pushed the cpp_result_release_raw branch from a70e297 to 3ffadec Compare May 21, 2019 09:20
@chfast chfast changed the title cpp: Rename result::raw() to result::acquire_raw() cpp: Rename result::raw() to result::release_raw() May 21, 2019
@chfast chfast merged commit 37a5b8a into master May 21, 2019
@chfast chfast deleted the cpp_result_release_raw branch May 21, 2019 09:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants