Skip to content
This repository was archived by the owner on Feb 26, 2020. It is now read-only.

Conversation

@andresilva
Copy link

This is necessary to allow publishing to crates.io. Merge only after paritytech/rust-snappy#6 since we'll want to change our snappy dependency to the one we'll publish to crates.io.

@andresilva andresilva requested a review from dvdplm August 27, 2018 13:18
@andresilva
Copy link
Author

I bumped the version to 0.5, just because we're currently tracking RocksDB v5. I don't have any opinion on version number, let me know if you don't like it.

@andresilva
Copy link
Author

I also fixed some tests that were disabled.

Copy link

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm

Cargo.toml Outdated
version = "0.4.5"
authors = ["Tyler Neely <[email protected]>", "David Greenberg <[email protected]>"]
version = "0.5.0"
authors = ["Parity Technologies <[email protected]>"]
Copy link

Choose a reason for hiding this comment

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

I'd probably keep the original authors in there too, like authors = ["Tyler Neely <[email protected]>", "David Greenberg <[email protected]>, "Parity Technologies <[email protected]>"]

@dvdplm
Copy link

dvdplm commented Aug 27, 2018

Closes #16

@andresilva
Copy link
Author

andresilva commented Aug 27, 2018

Added the upstream authors as well. I still need to update the snappy-sys dependency (after we publish it) before we merge this.

@andresilva andresilva force-pushed the andre/rename-crate branch 7 times, most recently from 4af5a7b to 22bb6ff Compare August 31, 2018 01:18
Summary:
sysmacros.h should be included in OS_ANDROID build as well otherwise the compile would complain: error: use of undeclared identifier 'major'.
Fixes facebook/rocksdb#4231
Pull Request resolved: facebook/rocksdb#4232

Differential Revision: D9217350

Pulled By: maysamyabandeh

fbshipit-source-id: 21f4b62dbbda3163120ac0b38b95d95d35d67dce
Copy link

Choose a reason for hiding this comment

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

Is CARGO_CFG_TARGET_OS set automatically when cargo runs, i.e. is it a "standard" env var? What is the default?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it is set by cargo depending on the OS the current build is targeting. This would be the same as if cfg!(target_os = "android") {} but the cfg! macro doesn't do the right thing when cross-compiling.

Copy link

@tomaka tomaka Aug 31, 2018

Choose a reason for hiding this comment

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

Nit: shouldn't that be an expect("...") instead of unwrap_or_default()?

@andresilva andresilva merged commit f1252c8 into master Sep 3, 2018
@debris debris deleted the andre/rename-crate branch September 18, 2018 20:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants