Code review

[hhzl]

Please add notes, remarks about the code and the process.

[ghost]

If snap.svg library is AMD-aware (so if has some header doing define call), it likely exports the Snap object. So it should not be accessed via global, like:

| s bigCircle smallCircle |

s := Snap value: 300 value: 600.

but via loader, like:

| s snapLib bigCircle smallCircle |

snapLib := require value: 'snap.svg'.
s := snapLib value: 300 value: 600.

[ghost]

You can even load it dynamically completely, not putting it to deploy.js but actually loading it jit, like:

| s bigCircle smallCircle |

require value: #('snap.svg') value: [ :snapLib |
  s := snapLib value: 300 value: 600.

but the previous seems easier to read.

[hhzl]

@Herby

1

regarding the first comment: thank you, updated to use

| s snapLib bigCircle smallCircle |

snapLib := require value: 'snap.svg'.
s := snapLib value: 300 value: 600.

code

2

Regarding the second comment:

an argument in favor of adding it to deploy.js is that the library is included in the file the.js to be used for deployment.

[ghost]

Hannes Hirzel wrote:

@Herby https://github.com/herby

1

regarding the first comment: thank you, updated to use

|| s snapLib bigCircle smallCircle |

snapLib := require value: 'snap.svg'.
s := snapLib value: 300 value: 600.
|

code
https://github.com/hhzl/Amber-snapsvg-demo/blob/master/src/AmberSnapsvg.st

2

Regarding the second comment:

an argument in favor of adding it to |deploy.js| is that the library
is included in the file |the.js| to be used for deployment.

Oh yes, I forgot. Definitely.

[hhzl]

Thank you