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.

Sign up for GitHub

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

Jump to bottom

Fix game of life #229

Merged
merged 7 commits into from
May 14, 2018
Merged

Fix game of life #229

merged 7 commits into from
May 14, 2018

Conversation

huangjj27
Copy link
Contributor

@huangjj27 huangjj27 commented May 11, 2018 •
edited
Loading

  • update rand to github master, need to lock after new rand released
  • replace println! with console.log, need nll for more readable code
  • update example version and author message
  • change referenced js/app.js to game_of_life.js

@therustmonk
Copy link
Member

@huangjj27 Cool! Thank you for the PR 👍
I've created and released https://github.com/DenisKolodin/web_logger. Now we could use log crate instead of workarounds.
Could you update this PR to log dependency after #230 will be merged?

@huangjj27
Copy link
Contributor Author

All right, it’ll be done by tomorrow.

@huangjj27
Copy link
Contributor Author

@deniskolodin Some advice is deperating the ConsoleService in yew when the web_logger stablized ,

@huangjj27
Copy link
Contributor Author

build failed due to #231 in the showcase and a compacity problem in other target is found.
Asking for seperate target build and test stage in .travis.yml.

@therustmonk
Copy link
Member

Let me know when you finish this PR, I want to rebase and merge it.

@huangjj27
Copy link
Contributor Author

@deniskolodin I think my job is done, but there is some campatibility issue from rand crate blocking. So just waiting.

@huangjj27
Copy link
Contributor Author

I think we have to wait for this feature rust-lang/cargo#1197 to merge this PR

@therustmonk
Copy link
Member

I've rebased this PR and added some fixes.
I will merge it when dependent crates will be fixed and all tests will be passed.
Thank you 👍

@huangjj27
Copy link
Contributor Author

problem is fixed but PR is waiting for merging

@huangjj27
Copy link
Contributor Author

@deniskolodin I found that you changed the rand to version 0.5-pre.1, which doesn't include the fix. May need to update to next version before the final check.

@therustmonk
Copy link
Member

Fix hasn't merged and in any case we have to use private branch for checking. I'll do it now. Please, don't push updates to this PR.

huangjj27 and others added 6 commits May 13, 2018 10:15
@huangjj27 @therustmonk
bugfix(rand): add stdweb feature
8a84565 
This is a temporary bugfix to make the game of life example work.
It traces the newest source code in order to use the stdweb feature
to make it work. The version of rand crate should be locked once
the 0.5 version is released on crates.io

close #227
@huangjj27 @therustmonk
bugfix(logging): use browser console for logging
69aa458 
As the stdout is not included in wasm32-unknwon-unknown target,
replace the println! macros with yew's ConsoleService.

fix #228
@huangjj27 @therustmonk
chore(version): update version message
f1daf47
@huangjj27 @therustmonk
refactor(logging): replace console with web-logger
94d86af
@huangjj27 @therustmonk
bugfix(index): cargo web command consistence
1f895e2 
Notice that the "cargo web start" command indexing "js/app.js"
while the output "cargo web deploy" command have only "game_of_life.js".
Tried running proving the latter referencing also work with "start"
command.
@therustmonk
Use thread_rng instead of deprecated random function
90a02de
@therustmonk
Copy link
Member

PR rust-random/rand#444 wokrs

@huangjj27
Copy link
Contributor Author

huangjj27 commented May 14, 2018 •
edited
Loading

@deniskolodin the PR is just merged. Now we can test with the newest source code of rand.

@therustmonk
Copy link
Member

I've updated the dependency to the master branch. If test will be passed I will merge it today and won't wait for the release of rand. The support in master is enough for this example.

@therustmonk
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request May 14, 2018
@bors @huangjj27 @therustmonk
Merge #229
0617687 
229: Fix game of life r=DenisKolodin a=huangjj27

- update rand to github master, need to lock after new rand released
- replace `println!` with `console.log`, need nll for more readable code
- update example version and author message
- change referenced `js/app.js` to `game_of_life.js`

Co-authored-by: huangjj27 <[email protected]>
Co-authored-by: Denis Kolodin <[email protected]>
@bors
Copy link
Contributor

bors bot commented May 14, 2018

Build succeeded

@bors bors bot merged commit 29f018f into yewstack:master May 14, 2018
@huangjj27 huangjj27 deleted the fix-game-of-life branch May 15, 2018 01:36
@therustmonk therustmonk mentioned this pull request Jun 8, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@huangjj27 @therustmonk