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

Fixed Readme for AssertFunction Call Case #523

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

kudla
Copy link
Contributor

@kudla kudla commented Jul 4, 2023

No description provided.

@@ -171,8 +171,8 @@ There are 2 approaches:
- Using [AssertFunction()](https://pkg.go.dev/github.com/dop251/goja#AssertFunction):
```go
const SCRIPT = `
function f(param) {
return +param + 2;
function sum(a, b) {
Copy link
Owner

Choose a reason for hiding this comment

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

This was changed by #511 to make it more consistent with the following example. What I didn't realise is that the rest of the example was not changed. I think it's better to do that instead.

Copy link
Contributor Author

@kudla kudla Jul 7, 2023

Choose a reason for hiding this comment

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

Looks it #511 wrapped script body into a constant and adjusted the function body it self. Yet didn't sync the refer usage part.

With following the consistency idea I've turned both the examples to use the same more meaningful function sum (which might be more clear to grasp the logic) with the same notation and the same usage holder variables.

@kudla kudla force-pushed the fix-assert-function-readme branch from 473fd9f to 11c1ee7 Compare July 7, 2023 13:54
@dop251 dop251 merged commit 636fdf9 into dop251:master Jul 7, 2023
Gabri3l pushed a commit to Gabri3l/goja that referenced this pull request Sep 20, 2023
* Fixed Readme for AssertFunction Call Case

* Made Consistent Both func Variable Name and Logic for Function Calling Examples

(cherry picked from commit 636fdf9)
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