Skip to content

Make more service's methods static #1382

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

Merged
merged 1 commit into from
Jul 9, 2020
Merged

Make more service's methods static #1382

merged 1 commit into from
Jul 9, 2020

Conversation

jplatte
Copy link
Contributor

@jplatte jplatte commented Jul 6, 2020

Description

Continuation of #1313.

Checklist:

  • I have run ./ci/run_stable_checks.sh
  • I have reviewed my own code
  • I have added tests – no because I haven't added anything new

@jplatte jplatte mentioned this pull request Jul 6, 2020
3 tasks
@jstarry
Copy link
Member

jstarry commented Jul 8, 2020

Thanks @jplatte looks like you need to give the &str error type a 'static lifetime to make CI happy. I think it wasn't needed before due to lifetime elision rules with &self

@jplatte
Copy link
Contributor Author

jplatte commented Jul 8, 2020

@jstarry I was wondering why the CI script exited immediately but decided to just rely on my editor integration. It didn't work for stdweb apparently. Would be nice if there was an easier way to run the CI script, one that doesn't require setting the default rustup toolchain to stable.

@jstarry
Copy link
Member

jstarry commented Jul 9, 2020

Checks have different behavior on different rust versions. I like the consistency of running checks on stable. Maybe we need a warning in the script if you run without stable?

@jstarry jstarry merged commit 105422b into yewstack:master Jul 9, 2020
@jplatte
Copy link
Contributor Author

jplatte commented Jul 9, 2020

That still would make it quite annoying to have to change defaults. I was thinking adding +stable to the commands and conditionally running it in CI (assuming you have non-stable CI jobs).

@jstarry
Copy link
Member

jstarry commented Jul 9, 2020

Good idea! We can move the stable check into a wrapper script that is only used for CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants