Skip to content

Adopt cargo-make#1541

Merged
siku2 merged 13 commits into
yewstack:masterfrom
siku2:adopt-cargo-make
Sep 14, 2020
Merged

Adopt cargo-make#1541
siku2 merged 13 commits into
yewstack:masterfrom
siku2:adopt-cargo-make

Conversation

@siku2
Copy link
Copy Markdown
Member

@siku2 siku2 commented Sep 1, 2020

Description

Updates all the remaining things to adopt cargo-make as the new task runner instead of unportable bash scripts.
Closes #1504

TODO

  • tasks.bench for yew is ignored by the flow
  • tasks.test for yew-macro should properly enforce the correct Rust version
  • find out how many other things I accidentally broke

@siku2 siku2 mentioned this pull request Sep 5, 2020
13 tasks
also, that Geckodriver issue seems to be resolved
@siku2 siku2 linked an issue Sep 9, 2020 that may be closed by this pull request
@siku2 siku2 marked this pull request as ready for review September 10, 2020 17:00
Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
@siku2 siku2 merged commit b6e98ad into yewstack:master Sep 14, 2020
@siku2
Copy link
Copy Markdown
Member Author

siku2 commented Sep 14, 2020

Partially resolves #1368 too. Next up, updating the examples.

@siku2 siku2 deleted the adopt-cargo-make branch September 14, 2020 13:39
Comment thread yew-macro/Makefile.toml
dependencies = ["install-msrv-toolchain"]
script = [
"""
cargo +${MSRV} test
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@siku2 tasks can set the toolchain to work with using the toolchain attribute.
The value of that attribute is used by rustup to install if needed.
so you could do

[tasks.test]
toolchain = "1.45.2"
command = "cargo"
args = [ "test" ]

see more info at https://github.com/sagiegurari/cargo-make#usage-toolchain

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh wow, you even used it in the original version and I still totally missed it 😅
I think I got confused by the rust_version condition which does something totally different.

I originally wanted to ask you for a review on this but I thought that would be an unreasonable thing to ask.
Anyway, thanks for pointing this out. That makes it a lot cleaner!

Copy link
Copy Markdown
Member Author

@siku2 siku2 Sep 22, 2020

Choose a reason for hiding this comment

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

@sagiegurari Finally got around to this! I noticed that unlike my little script, the toolchain setting won't attempt to automatically download the toolchain.
The error it generates is nice enough so I'm still going to use it but cargo-make already automatically downloads packages (which takes significantly longer because they need to be compiled) so I would expect it to do the same for toolchains. What do you think?

siku2 added a commit to siku2/yew that referenced this pull request Sep 22, 2020
siku2 added a commit that referenced this pull request Sep 22, 2020
* remove getrandom from game_of_life

I know the wasm-bindgen feature is deprecated but this is more convenient for now
and all the other examples use it this way as well.
We can update all of them in one go when rand 0.8 rolls around

* update todomvc README

* clean up yew-macro makefile

See: #1541 (comment)

* Use Firebase Hosting
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.

Proposal: Replace bash scripts with a Rust CLI

3 participants