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

Add compatibility with 1.34 #3

Merged
merged 10 commits into from Jul 4, 2020
Merged

Add compatibility with 1.34 #3

merged 10 commits into from Jul 4, 2020

Conversation

jyn514
Copy link
Contributor

@jyn514 jyn514 commented Jul 4, 2020

This PR is best reviewed commit-by-commit. See individual commits for a rationale of the changes.

Since any references cannot be used after RodeoReader is dropped, there
is no need to drop them explicitly. Removing the drop impl allows
destructuring the reader in into_resolver, simplifying the code.
Same logic as RodeoResolver
No need to call `from_raw_parts` twice.
@jyn514
Copy link
Contributor Author

jyn514 commented Jul 4, 2020

I want to add 1.34 to CI but I don't know how to use github actions.

@jyn514
Copy link
Contributor Author

jyn514 commented Jul 4, 2020

It looks like this doesn't actually run 1.34 and just downloads it temporarily before switching the default to nightly.

@Kixiron
Copy link
Owner

Kixiron commented Jul 4, 2020

I think the best thing to do is either add a separate msrv job that just runs check on 1.34 or to use cargo-msrv in another job

Copy link
Owner

@Kixiron Kixiron left a comment

Choose a reason for hiding this comment

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

Thank you for contributing! Cleaning up the code is much appreciated (and much needed), it'll be good to go after the CI issues get ironed out (Publishing a release may take a week or so though)

I think this should work for testing against 1.34.0

msrv:
  name: MSRV
  runs-on: ubuntu-latest

  strategy:
    fail-fast: false

  env:
    RUST_BACKTRACE: 1
    CARGO_INCREMENTAL: 0

  steps:
    - uses: actions/checkout@v2
    - uses: actions-rs/toolchain@v1
      with:
        profile: minimal
        toolchain: 1.34.0
        override: true
  
    - name: Check
      uses: actions-rs/cargo@v1
      with:
        command: check
        args: --features multi-threaded
    
    - name: Test
      uses: actions-rs/cargo@v1
      with:
        command: test
        args: --features multi-threaded

@jyn514
Copy link
Contributor Author

jyn514 commented Jul 4, 2020

Testing won't work because criterion isn't compatible with 1.34. The rest is super helpful though!

@jyn514
Copy link
Contributor Author

jyn514 commented Jul 4, 2020

--features multi-threaded also won't work because dashmap isn't compatible with 1.34.

error[E0658]: use of unstable library feature 'map_get_key_value' (see issue #49347)
   --> /home/joshua/.local/lib/cargo/registry/src/github.com-1ecc6299db9ec823/dashmap-3.11.5/src/lib.rs:695:37
    |
695 |         if let Some((k, v)) = shard.get_key_value(key) {
    |                                     ^^^^^^^^^^^^^

.github/workflows/main.yml Outdated Show resolved Hide resolved
@Kixiron
Copy link
Owner

Kixiron commented Jul 4, 2020

Thanks!

@Kixiron Kixiron merged commit 5bdbc87 into Kixiron:master Jul 4, 2020
@jyn514 jyn514 deleted the 1.34 branch July 4, 2020 23:14
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.

None yet

2 participants