Re-export Parity struct

[sanket1729]

pub struct Parity is under a private module key and not re-exported in lib.rs . It is therefore not
possible to use it downstream.

[sanket1729]

Might need another point release. I wish rust compiler would warn about these things.

cc @apoelstra , @elichai.

[sanket1729]

We can also make key mod public. I don't know which one is better or the preferred way to do things.

[tcharding]

oooo, my mistake. Thanks for catching this. Another option would be to use a wildcard statement like we do for context (pub use key::*).

[tcharding]

Might need another point release.

I think 0.21.0 is totally broken because of this and will need to be yanked from crates.io Hopefully no one is using the taproot stuff yet so we won't have too much egg on our face.

I wish rust compiler would warn about these things.

I think regression/integration tests is what should have caught this. Adding to my TODO list.

[thomaseizinger]

I think regression/integration tests is what should have caught this. Adding to my TODO list.

Moving the tests to tests/ is also a good way of checking that the public API of the crate is actually usable.

[elichai]

ACK e595b39

[apoelstra]

I think 0.21.0 is totally broken because of this and will need to be yanked from crates.io Hopefully no one is using the taproot stuff yet so we won't have too much egg on our face.

Sure, though I'll publish 0.21.1 before yanking it. I think the keypair related stuff is "totally broken" but not the rest of the library, so best not to make non-schnorr people undo any integration work they might've started in the last 24 hours.

[apoelstra]

ACK e595b39

[apoelstra]

Published and tagged.

Yanked 0.21.0.

[sanket1729]

Another error. Sorry about that. I think we also need basic derives for Parity :( . I think this highlights the need for the release process. Maybe perhaps have a RC for a couple days.

[apoelstra]

Personally, I think it's fine to treat point releases as RCs, and to have a few iterations of this whenever we do a major release.

@tcharding what do you think?

[tcharding]

Personally, I think it's fine to treat point releases as RCs

This is an interesting idea, I'd never thought of it before. Is anyone else in the Rust community doing this (or other users of semvar in general)? I think before 1.0 we might be able to get away with it but it seems a little bit cheeky. Before we release 1.0 we should have a proper release process with a release candidates, feature freeze window, etc. so that we don't treat users as beta testers.

Another error. Sorry about that. I think we also need basic derives for Parity

Wow, I really did botch this one. Missing derives is something Clippy can definitely help us with, at least for Copy and Debug. I'll raise a PR to add attributes to configure Clippy to prevent this happening in the future.

[tcharding]

@apoelstra please see #362 adding derives. Please do another point release at some stage and add to my tab 'Tobin owes Andrew one pint of the finest'.

[apoelstra]

Adding attributes to Clippy would be awesome.

After a bit of thought, I think there's a lot of value in releasing rcs for rust-secp, bitcoin_hashes, etc., until we at least know that we can even compile rust-bitcoin with the new features that we've added. I agree that it's cheeky, at the very least, to use point releases for this purpose.

And I'll never say no to free beer ;) but I really should've caught this as well.

[tcharding]

Adding attributes to Clippy would be awesome.

We can use

#![warn(missing_debug_implementations)]
#![warn(missing_copy_implementations)]

(I made a mistake above, these are inbuilt Rustc lints not Clippy.)

Although they only catch missing Copy and Debug this can still highlight for devs the line of code with a derive issue.

On this topic, does rust-secp256k1 want to move towards enforcing the addition of opportunistically adding derives to all types as is suggested doing in rust-bitcoin in the proposed CONTIRUBTING.md document? I.e.

#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct Foo {}

[apoelstra]

I think that's reasonable for rust-secp, yeah. I'm sure there are exceptions but we should default to having them.