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

visualization blog edits #66

Merged
merged 10 commits into from
Sep 24, 2020
Merged

visualization blog edits #66

merged 10 commits into from
Sep 24, 2020

Conversation

jspaaks
Copy link

@jspaaks jspaaks commented Sep 24, 2020

This PR adds a couple of edits on the visualization blog, mostly style and rephrase and such.

@jspaaks jspaaks changed the title Jspaaks visualization edits visualization blog edits Sep 24, 2020
@jspaaks jspaaks marked this pull request as ready for review September 24, 2020 11:36
@jspaaks jspaaks requested a review from sverhoeven September 24, 2020 11:36
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

Looks good to me. Sentences are smoother now.

Lots more quotes, some sentences now have a bit to many quotes for me. Feels like every odd word is quoted. Especially line 22, 50, 126, 127

Can you correct the links on the plot images so they point to renamed directory?

@@ -4,8 +4,8 @@
#include "newtonraphson.hpp"

int main() {
double initial_guess = -4;
double tolerance = 0.001;
float initial_guess = -4;
Copy link
Member

Choose a reason for hiding this comment

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

Nice inconsistency find

Emscripten can handle simple types like float and int, but needs help exposing more complex types to JavaScript like the iterations property.
We need to use [value_object](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#value-types) to expose the Iteration struct and [register_vector](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#built-in-type-conversions) as the iterations property type.
Emscripten can handle simple types like `float` and `int`, but needs help exposing more complex types to JavaScript like the `iterations` property.
We need to use [`value_object`](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#value-types) to expose the `Iteration` `struct` and [`register_vector`](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#built-in-type-conversions) as the `iterations` property type.
Copy link
Member

Choose a reason for hiding this comment

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

Links are already rendered differently, they do not need more highlighting

Suggested change
We need to use [`value_object`](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#value-types) to expose the `Iteration` `struct` and [`register_vector`](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#built-in-type-conversions) as the `iterations` property type.
We need to use [value_object](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#value-types) to expose the `Iteration` `struct` and [register_vector](https://emscripten.org/docs/porting/connecting_cpp_and_javascript/embind.html#built-in-type-conversions) as the `iterations` property type.

Copy link
Author

Choose a reason for hiding this comment

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

strictly speaking I don't care to highlight them, but I want to emphasize that I'm referring to something in the code. It's your call though.


```shell
emcc -I. -o newtonraphson.js -Oz -s MODULARIZE=1 \
-s EXPORT_NAME=createModule --bind newtonraphson.cpp bindings.cpp
```

<!--
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I forgot about that one, nice to see #65

@jspaaks
Copy link
Author

jspaaks commented Sep 24, 2020

Lots more quotes, some sentences now have a bit to many quotes for me. Feels like every odd word is quoted. Especially line 22, 50, 126, 127

Agree, but not quoting them is not the right solution to me

@jspaaks jspaaks requested a review from sverhoeven September 24, 2020 13:57
Copy link
Member

@sverhoeven sverhoeven left a comment

Choose a reason for hiding this comment

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

I can not think of another way to tell the string is part of a code snippet.
Lets keep the quotes for consistencies sake.

@jspaaks jspaaks merged commit cfc6022 into master Sep 24, 2020
@jspaaks jspaaks deleted the jspaaks-visualization-edits branch September 24, 2020 14:10
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