`cargo publish` does not allow publishing a crate without all
dependencies having explicit version numbers. We discovered recently
that having patching the root manifest was annoying when downstream
testing so we removing the version numbers in #4284 in favor of `path`.
It turns out we can include both.
Props to nyonson for discovering that.
ref: https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations
Use an explicit version number to the `internals` dep in `units`.
Currently we only need the `derive` feature of `serde` in test code.
Observe:
- We do not need the error testing logic because `ParseAmountError` is
already exhaustively tested.
- The rest of the `serde` test logic in `amount` can be done using the
public API so it can be moved to the integration test directory.
Move the unit test code to `tests/` excluding the error testing logic.
Remove the `derive` feature from the `serde` dependency. Add a
`dev-dependency` on `serde` that enables the `derive` feature.
f9be30ddbe units: Fix `missing_errors_doc` clippy lint (Jamil Lambert, PhD)
Pull request description:
Change the `missing_errors_doc` clippy lint to `warn`.
Allow `missing_errors_doc` in `amount/serde.rs` and `fee_rate/serde.rs`. Add missing `# Errors` sections to rustdocs where the lint gives a warning.
One of the TODO lints in Issue https://github.com/rust-bitcoin/rust-bitcoin/issues/3825
ACKs for top commit:
tcharding:
ACK f9be30ddbe
apoelstra:
ACK f9be30ddbe5c0837ab3e408dfadabc6c6cd2068e; successfully ran local tests
Tree-SHA512: 8039804ab86c18dceadb425c8531cd4064431393367c6053249e00386f48998d8d84a3aee6ad139e7e2ca3ac3c94e05ee694d72270bf285f6b90d0ff821e622e
29f1a4613a Enable `ref_option` lint and allow individually (Jamil Lambert, PhD)
Pull request description:
The last `TODO` in #3825
Allow the `ref_option` lint in the cases where the lint wants us to change the API to ```use `Option<&T>` instead of `&Option<T>` ```
The other option would be to keep the lint as `allow` and add a comment to `Cargo.toml` as to why we are allowing it like the other cases that are allowed.
Close#3825
ACKs for top commit:
apoelstra:
ACK 29f1a4613a090cd23baded1ef577aa4e3da8d33f; successfully ran local tests; nice!
tcharding:
ACK 29f1a4613a
Tree-SHA512: 181d617060e8ae0e75f033b1f356106fc89bdee9841085c889b775e82711c147e5282ecb0e3bf80cdb058034365bbfc7289c029ba0bf8b0b0f26d22505c63aa2
Change the lint to `warn` in `units/Cargo.toml`.
Allow `missing_errors_doc` in `amount/serde.rs` and `fee_rate/serde.rs`.
Add missing `# Errors` sections to rustdocs where the lint gives a
warning.
Add an integration test the verifies we have serde traits implemented
for all the main crate types (excl. errors and helper structs).
This also acts as a regression test.
Add all the pedantic lints to the repository by way of the repository
manifest. Then enable these lints in the `units` manifest.
Some things worth mentioning:
- Fix `needless_pass_by_value` by adding derives to `FormatOptions`.
- Fix lint `cast_lossless` using `cargo clippy --fix``
- While fixing `lint enum_glob_use` introduce a new style to the
codebase; import enums using a single character. Doing so prevents
namespace clashes, improves clarity, and maintains terseness.
Audit:
Use the following lints locally and audit all the warnings, they produce
many false positives so we can't enable them permentently.
- `cast_possible_truncation`
- `cast_possible_lint`
- `cast_sign_loss`
463fbd16f1 Update to arbitrary v1.4 (Tobin C. Harding)
Pull request description:
Out with the old in with the new, update to the latest minor version of arbitrary.
No real reason to do this except that I wanted to add the minor version instead of using just `1` so that we don't accidentally pull a new minor version in during a patch release (after we 1.0).
(I'm unsure if this is necessary and did not test it against MSRV.)
ACKs for top commit:
apoelstra:
ACK 463fbd16f1431febe7e4fc50abd7415886542109; successfully ran local tests
Tree-SHA512: 88913f993b97abba224ba94ee9d5057894bb7142fef07e557de03a423da314a9f21632932fb64c4c27c676051bbb6e90d9b90bfa944af810e834bf42707d3cff
Out with the old in with the new, update to the latest minor version of
arbitrary.
No real reason to do this except that I wanted to add the minor version
instead of using just `1` so that we don't accidentally pull a new minor
version in during a patch release (after we 1.0).
Sometimes JSON parsers may munge floats. Instead of using `f64` we can
serialize BTC amounts as strings.
Includes addition of `alloc` feature gate to `DisplayFullError` to
remove lint warnings when building with `--no-default-features`.
Close: #894
In preparation for releasing `units v0.2.0` bump the version number,
add a changelog entry, update the lock files, and depend on the new
version in all crates that depend on `units`.
In preparation for releasing `internals v0.4.0` bump the version number,
add a changelog entry, update the lock files, and depend on the new
version in all crates that depend on `internals`.
The version 1.63 satisfies our requirements for MSRV and provides
significant benefits so this commit bumps it. This commit also starts
using some advantages of the new MSRV, namely namespaced features, weak
dependencies and the ability to use trait bounds in `const` context.
This however does not yet migrade the `rand-std` feature because that
requires a release of `secp256k1` with the same kind of change - bumping
MSRV to 1.63 and removing `rand-std` in favor of weak dependency.
We currently duplicate the serde_round_trip macro in `units` and
`bitcoin`, this is unnecessary since it is a private test macro we can
just throw it in `internals`.
While we are at it lets improve the macro by testing a binary encoding
also, elect to use the `bincode` crate because we already have it in
our dependency graph.
Add `test-serde` feature to `internals` to feature gate the macro and
its usage (preventing the transient dependency on `bincode` and
`serde_json`).
In a2b019f82 it was claimed that losing the input string without alloc
is a bug. It is not, because allowing no-alloc is desired and there's
no way to have the input string otherwise so we just accept it'll be
missing and modify the messages accordingly. The commit that forced
alloc was also horribly misleading since it kept the `alloc` feature but
it makes this one easier.
Note that input string is still present by default in all configurations
except for no-alloc.
The package metatadata never changes and is not necessary to look at
basically ever, put it down the bottom of the manifest out of the way.
Helps to keep features and dependencies closer together.
Refactor only, no logic changes.
We just did a minor bug fix to error code in the `amounts` module. This
change did not effect the public API but improved the display of two
error types from that module.
In preparation for doing a point release bump the version number and add
a changelog entry.
We have 2 crates that require an allocator, `bitcoin` and `base58ck` -
these crates should enable the "alloc" feature when depending on
`internals`.
For `units` we use the `internals::error::InputString` but do not enable
the "alloc" feature - this is a bug, it means that the parsed string is
being lost from the error types that use `InputString`.
Enable "alloc" for `bitcoin`, `base58ck`, and `units`.
- `bitcoin` and `base56ck` is just for good measure so we don't get
bitten later on.
- `units` is a bug fix and requires a point release.
Tobin C. Harding
Eval EXEC
merge-script
Jamil Lambert, PhD
yancy
Martin Habovstiak