Skip to content

Example build hardening#1430

Merged
siku2 merged 3 commits into
yewstack:masterfrom
p-alik:example-build-hardening
Jul 22, 2020
Merged

Example build hardening#1430
siku2 merged 3 commits into
yewstack:masterfrom
p-alik:example-build-hardening

Conversation

@p-alik
Copy link
Copy Markdown
Contributor

@p-alik p-alik commented Jul 20, 2020

Description

The script examples/build.sh should check if approached example (defined by $1) exists. It should use wasm-pack for todomvc example.

I'm aware about Proposal: Replace bash scripts with a Rust CLI, but as long as the script exists in the repository, it should support build of all examples.

Testing instructions

$ for d in ./*; do bn=basename $d; if [ ! -d $d ] || [ $bn = "common" ] || [ $bn = "static" ] || [ $bn = "server" ]; then continue; fi; echo ">$bn<"; ./build.sh $bn; done

I couldn't build server example for same of openssl dependencies yet.

Copy link
Copy Markdown
Member

@siku2 siku2 left a comment

Choose a reason for hiding this comment

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

I totally agree that we need to keep the examples working while #1418 is in the works. Thanks for taking the time to update the script.

There's currently another pull request which addresses similar problems in the script: #1417.
I think our best course of action is to wait until that PR is merged and then revisit this one, what do you think?

Comment thread examples/build.sh Outdated
Comment thread examples/build.sh Outdated
Comment thread examples/build.sh Outdated
Comment thread examples/build.sh Outdated
@p-alik
Copy link
Copy Markdown
Contributor Author

p-alik commented Jul 21, 2020

I think our best course of action is to wait until that PR is merged and then revisit this one, what do you think?

Indeed, I've to wait for #1417 is merged.

@siku2
Copy link
Copy Markdown
Member

siku2 commented Jul 22, 2020

@p-alik Happy to report that #1417 has been merged!
Most of the changes here have become redundant but there is still one thing I would love to add to Yew.
I'm talking about the check for the empty $EXAMPLE variable.

I would also like to take it one step further by adding a helpful error message if the user tries to run an example that doesn't exist. Currently the error message looks like this, which leaves a lot to be desired:

./run_example.sh: line 63: cd: yew/examples/abc: No such file or directory
[FAIL] Command: "cd "$SRCDIR/$EXAMPLE"" exited with exit code: 1

If you don't have time to make these changes let me know, we'll just open a new issue.

@p-alik p-alik force-pushed the example-build-hardening branch from 4e3bf87 to 87fd987 Compare July 22, 2020 12:28
Comment thread examples/run_example.sh Outdated
the script exits if without example name parameter
or if example directory doesn't exist
@p-alik p-alik force-pushed the example-build-hardening branch from 87fd987 to 572f97e Compare July 22, 2020 12:40
Comment thread examples/run_example.sh Outdated
Comment thread examples/run_example.sh Outdated
p-alik and others added 2 commits July 22, 2020 15:12
Co-authored-by: Simon <simon@siku2.io>
Co-authored-by: Simon <simon@siku2.io>
@siku2 siku2 merged commit fb27791 into yewstack:master Jul 22, 2020
@siku2
Copy link
Copy Markdown
Member

siku2 commented Jul 22, 2020

Thanks for indulging me, @p-alik!

@p-alik p-alik deleted the example-build-hardening branch July 22, 2020 15:40
jstarry pushed a commit that referenced this pull request Aug 16, 2020
* run_example.sh checks example existens

the script exits if without example name parameter
or if example directory doesn't exist

* Update examples/run_example.sh

Co-authored-by: Simon <simon@siku2.io>

* Update examples/run_example.sh

Co-authored-by: Simon <simon@siku2.io>

Co-authored-by: Simon <simon@siku2.io>
teymour-aldridge pushed a commit to teymour-aldridge/yew that referenced this pull request Sep 1, 2020
* run_example.sh checks example existens

the script exits if without example name parameter
or if example directory doesn't exist

* Update examples/run_example.sh

Co-authored-by: Simon <simon@siku2.io>

* Update examples/run_example.sh

Co-authored-by: Simon <simon@siku2.io>

Co-authored-by: Simon <simon@siku2.io>
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