Skip to content

Conversation

@rossberg
Copy link
Member

@rossberg rossberg commented Apr 7, 2016

@lukewagner
Copy link
Member

lgtm, yes, we should update/add memory_size too

@rossberg
Copy link
Member Author

rossberg commented Apr 7, 2016

Okay, I added memory_size to this PR, PTAL.

@lukewagner
Copy link
Member

lgtm. One question I just remembered, and the reason I didn't simply throw in the memory_size update with design/#629, is that "size" doesn't sound quite right if the returned unit is pages. That could be a separate issue, though.

@rossberg
Copy link
Member Author

rossberg commented Apr 7, 2016

Agreed. current_memory perhaps?

On 7 April 2016 at 17:34, Luke Wagner notifications@github.com wrote:

lgtm. One question I just remembered, and the reason I didn't simply throw
in the memory_size update with design/#629
WebAssembly/design#629, is that "size" doesn't
sound quite right if the returned unit is pages. That could be a separate
issue, though.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#273 (comment)

@lukewagner
Copy link
Member

Yes, that has a nice symmetry with grow_memory.

@rossberg rossberg changed the title Make grow_memory return pages Make grow_memory and return pages, and replace memory_size with current_memory Apr 8, 2016
@rossberg
Copy link
Member Author

rossberg commented Apr 8, 2016

Okay, updated with a rename memory_size -> current_memory. Corresponding design PR is WebAssembly/design#648. PTAL

@lukewagner
Copy link
Member

Great, lgtm (assuming no objections to design PR).

@rossberg
Copy link
Member Author

Landing with LGTM and no objections

@rossberg rossberg merged commit e0d12c9 into master Apr 18, 2016
@rossberg rossberg deleted the grow_memory branch April 18, 2016 10:55
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