Stop using try_trait in persistent_store #322
Conversation
But still make sure it passes tests with the most recent nightly.
It is too much instable.
| with: | ||
| submodules: true | ||
|
|
||
| - uses: actions-rs/toolchain@v1 |
There was a problem hiding this comment.
I wasn't seeing it but now we have twice the toolchain setup.
There was a problem hiding this comment.
My understanding is that we install:
- the frozen compiler with the prod target
- the most recent nightly (to detect breakage early)
And we can only do this with 2 toolchain "actions".
|
|
||
| - uses: actions-rs/toolchain@v1 | ||
| with: | ||
| toolchain: nightly | ||
| override: true |
There was a problem hiding this comment.
By default (i.e. when override is not set to true) the action will respect the rust-toolchain file. Therefore it shouldn't be necessary to specify we want nightly.
But this requires the git submodules to be checked-out (which you added on this PR)
There was a problem hiding this comment.
I think it's better to check that the code compiles with both versions which is why I keep nightly.
|
|
||
| - name: Unit testing of Persistent store library (release mode) | ||
| uses: actions-rs/cargo@v1 | ||
| with: | ||
| toolchain: nightly |
There was a problem hiding this comment.
Is it necessary? The rust-toolchain file points to a specific nightly already. Do you need to override with a more recent one? Because this would have the effect that we unit-test the library with a different toolchain than the one we build against.
There was a problem hiding this comment.
I see 3 options:
- test only with frozen compiler
- test only with nightly
- test with both
I think they should be all equivalent (as long as the code compiles on both versions). First I wanted to do a matrix but it's not as simple because we can't name the rust-toolchain toolchain (in a maintainable way). But I can try again if we prefer to do everything (build + test) on both versions.
Prod is already built by opensk test.
|
Actually I just remembered why I was only using nightly, it's because opensk build already checks that the library builds. So it's enough to just run the tests. |
|
Don't forget to also issue a PR to the bugfix branch :) |
|
Actually, neither the bugfix nor the stable branch have this issue. It was introduced by #238 which was only on the develop branch. |
Fixes #320.