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

Migration to yew 0.18.0 #20

Merged
merged 4 commits into from
Jul 26, 2021
Merged

Migration to yew 0.18.0 #20

merged 4 commits into from
Jul 26, 2021

Conversation

Pscheidl
Copy link
Contributor

@Pscheidl Pscheidl commented May 31, 2021

Makes the components usable with Yew 0.18.0

release workflow

We follow semver for versioning this system.

  • update Cargo.toml version.
  • ensure CI completes successfully.
  • add a new tag to the repo matching the new Cargo.toml version. Either via git tag or via the Github UI.
    • all release tags should start with the letter v followed by a semver version.
  • CI is configured for release tags and will create a new Github release.
  • once CI for the tag has finished successfully, update the new release page with details on the changes made, which should reflect the content of the CHANGELOG.md.

@ppiotr3k
Copy link

@thedodd anything preventing from launching the CI workflow and moving forward on this PR? 🙏

Not being able to work using Yew 0.18 is personally a great pain, especially as Yew Documentation only addresses 0.18 now, that Yew Router has been greatly improved to work with it, etc.

@thedodd
Copy link
Owner

thedodd commented Jun 14, 2021

@ppiotr3k hey there. Just as a quick note to address timing, you don't actually need to wait for an official release to go out. You can have your Cargo.toml point to the forked source. That way you can immediately start using the updated code.

@Pscheidl
Copy link
Contributor Author

@ppiotr3k
ybc = { git = "https://github.com/Pscheidl/ybc/", branch= "yew-0.18.0" }

@Pscheidl
Copy link
Contributor Author

@thedodd Clippy righteously complained about duplicated clones. Corrected.

@ppiotr3k
Copy link

ppiotr3k commented Jun 19, 2021

@thedodd @Pscheidl thanks for both your replies 🙇

Actually I figured out there are uncovered changes from moving to Yew 0.18 in @Pscheidl's branch - which in all cases helps moving forward in upgrading to Yew 0.18. I therefore started PR #21 in order start having some tests, with the idea to ease on the upgrade path.

While #21 just proposes a starting point, leveraging existing Rustdoc examples for tests, it already shows IMHO a few other required fixes due to changes in Yew 0.18. I have already started more Rustdoc examples, to both increase documentation and provide test material, but I'm awaiting feedback on #21 to ensure this is an effort worth being taken 🙂

@Pscheidl
Copy link
Contributor Author

@thedodd Waiting for CI to be ran. Thank you.

@Follpvosten
Copy link
Contributor

What's going on here? I'd like to upgrade to 0.18 soon and I don't really want a git dependency or my own fork (although I'm considering it)

Copy link
Owner

@thedodd thedodd left a comment

Choose a reason for hiding this comment

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

@Pscheidl thank you for the PR! Sorry that it has taken me so long to review it. Just a few small items to change/discuss.

Cargo.toml Outdated Show resolved Hide resolved
src/form/select.rs Outdated Show resolved Hide resolved
Copy link
Owner

@thedodd thedodd left a comment

Choose a reason for hiding this comment

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

Everything looks great! Looks like CI is failing due to rustfmt lints being issued. This is quite likely due to rust/cargo version updates since the last release. You should be able to automatically incorporate such changes by running rustfmt exactly as CI does (it uses the rustfmt.toml in this repo).

Once that is done and CI is green, we'll be ready to merge this PR and I'll cut a new release.

@thedodd
Copy link
Owner

thedodd commented Jul 26, 2021

Disregard, I'll address the linting issues separately :). Cheers mate!

@thedodd thedodd merged commit 61d0133 into thedodd:master Jul 26, 2021
@Pscheidl
Copy link
Contributor Author

Ok, thank you @thedodd. Yup, that was it. I didn't check those details to be honest. I'm on 1.53 and I just use rustfmt automatically.

Thanks you.

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.

4 participants