search

seed-rs / seed-quickstart

Bare essentials to start a Seed app.
https://seed-rs.org/
101 stars 28 forks source link

Change type alias to struct #39

Closed zakaluka closed 3 years ago

zakaluka commented 3 years ago

For #29

MartinKavik commented 3 years ago

Hi @zakaluka and thanks for the PR.

Please make sure it's compilable, formatted and you should be able to remove some unnecessary Clippy attributes like #[allow(clippy::trivially_copy_pass_by_ref)]. Thanks!

zakaluka commented 3 years ago

I am not able to compile this - trying to figure out how to update the standback dependency because it fails to compile on 1.53 nightly. Will push any additional updates as required.

MartinKavik commented 3 years ago

It should work mainly on stable Rust. Nightly isn't required.

zakaluka commented 3 years ago

It looks like this is failing on Stable as well. Apparently standback had an issue where the wrong package (feature?) was being referenced: jhpratt/standback#11 . A new version has been released.

Should I try to use an older version of Rust to try and compile this?

zakaluka commented 3 years ago

Oddly enough ... when I build seed, it is using standback 0.2.15. However, I am not sure why the quickstart is trying to use 0.2.16

MartinKavik commented 3 years ago

I see. However it looks like they've fixed our older dependency issues and it's compilable for me when I delete Cargo.lock. Please delete your Cargo.lock and push the new one if it works, thanks.

zakaluka commented 3 years ago

Thanks @MartinKavik . Cargo.lock is fixed and a test from my fork was successful.

MartinKavik commented 3 years ago

Remove please #[derive(Default)] from Model and replace Model::default with

Model {
    counter: 0
}

and we should be ready to go. Reason: see Why don't implement Default and other standard traits block in https://seed-rs.org/0.8.0/model

MartinKavik commented 3 years ago

Could you finish it and squash commits when you have time? I would like to merge it and resolve the dependency problem. Thanks!

zakaluka commented 3 years ago

Will do. I will update all 3 areas.

zakaluka commented 3 years ago

I need to update the "Model" page on the guide. I will do that as part of this commit as well. Will let you know once I've tested it.

zakaluka commented 3 years ago

@MartinKavik 4 items have been updated, all linked to #29 in 3 separate pull requests. 2 commit checks are still running (seed and seed-rs.org). Please let me know if you see any other issues.

zakaluka commented 3 years ago

@MartinKavik also, in terms of squashing - I think you have that option when merging the PR. I tried to squash on seed but I'm not sure it did what you wanted.

MartinKavik commented 3 years ago

@MartinKavik also, in terms of squashing - I think you have that option when merging the PR. I tried to squash on seed but I'm not sure it did what you wanted.

The result should be one force-pushed commit. It's basically a two-clicks operation if you use something like GitKraken. However I can live with a couple of "weird" commits, I don't want to torture you with too many "administrative tasks". Almost all my repos allow only rebasing.

All PRs now look good if I don't overlook something. I'll merge them once all pipelines pass. Thanks!

zakaluka commented 3 years ago

I will try GitKraken next time - thanks for the pointer