Skip to content

Update logo - #497

Merged
notmandatory merged 1 commit into
bitcoindevkit:masterfrom
afilini:update-log
Dec 18, 2021
Merged

Update logo#497
notmandatory merged 1 commit into
bitcoindevkit:masterfrom
afilini:update-log

Conversation

@afilini

@afilini afilini commented Dec 10, 2021

Copy link
Copy Markdown
Member

Description

Update the logo in our readme

@RCasatta

Copy link
Copy Markdown
Contributor

this reminds me that is possible to change the logo also in the documentation, see https://www.davideaversa.it/blog/how-to-add-a-logo-in-rust-documentation/

@rajarshimaitra

Copy link
Copy Markdown
Contributor

ACK. Also on rust.doc as mentioned by @RCasatta .

@notmandatory

Copy link
Copy Markdown
Member

It's weird we're getting this clippy error, I think it has something to do with the macro running with nightly since this warning is only present in master/nightly:

https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding

@notmandatory

notmandatory commented Dec 11, 2021

Copy link
Copy Markdown
Member

I'm a bit stumped on this no_effect_underscore_binding issue.. I get the error with clippy 1.46.0, 1.47.0, 1.48.0.. those are all I tried. But I don't get the error with 1.56 or nightly.

Any ideas?

Reproduce with:

cargo +1.46.0 clippy --all-targets --features async-interface --no-default-features

@rajarshimaitra

rajarshimaitra commented Dec 11, 2021

Copy link
Copy Markdown
Contributor

I am not getting the error in 1.46.0, 1.47.0 and 1.56.0.

Though I am getting a new clippy error in stable and nightly (1.57.0 and 1.59.0). Probably that's due to some recent changes in 1.57.0.

$ cargo +stable clippy --all-targets --features async-interface --no-default-features -- -D warnings
    Checking bdk v0.14.1-dev (/home/raj/github-repo/bdk)
    Checking bitcoind v0.21.0
    Checking electrsd v0.13.0
error: manual implementation of `split_once`
  --> src/wallet/export.rs:99:5
   |
99 |     s.splitn(2, '#').next().map(String::from).unwrap()
   |     ^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Some(s.split_once('#').map_or(&*s, |x| x.0))`
   |
   = note: `-D clippy::manual-split-once` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once

error: this `impl` can be derived
   --> src/wallet/signer.rs:482:1
    |
482 | / impl Default for SignOptions {
483 | |     fn default() -> Self {
484 | |         SignOptions {
485 | |             trust_witness_utxo: false,
...   |
489 | |     }
490 | | }
    | |_^
    |
    = note: `-D clippy::derivable-impls` implied by `-D warnings`
    = help: try annotating `wallet::signer::SignOptions` with `#[derive(Default)]`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls

They seem unrelated to this error and straight forward, probably something we need to deal in next latest stable update.

But I can't reproduce the no_effect_underscore_binding error in my local.

Is it possible to rerun the CI tests?

@notmandatory

Copy link
Copy Markdown
Member

Now that #501 is in you can rebase on master to fix the CI issue.

@afilini

afilini commented Dec 18, 2021

Copy link
Copy Markdown
Member Author

this reminds me that is possible to change the logo also in the documentation, see https://www.davideaversa.it/blog/how-to-add-a-logo-in-rust-documentation/

I can do this in another PR once this is merged, so that i can point the url to the image hosted on github. Otherwise I have to upload it somewhere else just to have a publicly accessible url that i can use.

@notmandatory notmandatory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ACK 77bce06

@notmandatory
notmandatory merged commit 77bce06 into bitcoindevkit:master Dec 18, 2021
afilini added a commit to afilini/bdk that referenced this pull request Dec 23, 2021
As suggested in bitcoindevkit#497, add our logo to the docs as well
@afilini afilini mentioned this pull request Dec 23, 2021
3 tasks
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.

4 participants