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

chore: upgrade and bump some CI dependencies #95

Merged

Conversation

oleonardolima
Copy link
Collaborator

Description

This PR does some improvements on CI, these are some changes that I ended up doing on other refactoring and feature PRs (making them too convoluted), but had a specific CI scope so I'm moving them to a specific PR.

This PR does:

  • bump the actions/checkout@v3 to actions/checkout@v4.
  • adds two new jobs for fmt and clippy (clippy has been moved to a specific job).
  • fix the newly found fmt problems.
  • bump the rust edition to 2021.
  • adds .clippy.toml file with msrv=1.63.0.

Notes to the reviewers

I hope this PR reduces the scope convolution from the other ones #67 #93, and makes the review easier.

Changelog notice

  • Bump the actions/checkout@v3 to actions/checkout@v4.
  • Adds two new jobs for fmt and clippy (clippy has been moved to a specific job).
  • Multiple fixes for the newly found fmt problems.
  • Bump the rust edition to 2021.
  • Adds .clippy.toml file with msrv=1.63.0.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

@oleonardolima oleonardolima force-pushed the chore/upgrade-and-bump-ci branch from 150b86a to bcb66e6 Compare August 23, 2024 17:25
@coveralls
Copy link

coveralls commented Aug 23, 2024

Pull Request Test Coverage Report for Build 10657465976

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.09%) to 87.438%

Files with Coverage Reduction New Missed Lines %
src/lib.rs 1 97.08%
Totals Coverage Status
Change from base Build 10514244019: -0.09%
Covered Lines: 1058
Relevant Lines: 1210

💛 - Coveralls

@oleonardolima oleonardolima marked this pull request as ready for review August 26, 2024 19:26
@oleonardolima oleonardolima self-assigned this Aug 26, 2024
@oleonardolima oleonardolima added the ci Continuous Integration issues label Aug 26, 2024
@notmandatory
Copy link
Member

Concept ACK, overall this looks like the right thing to do. Just need to add rustfmt.toml. I've added a corresponding issue for the bdk project.

@oleonardolima oleonardolima force-pushed the chore/upgrade-and-bump-ci branch from 32dd561 to 18e459b Compare September 1, 2024 21:23
- rename the workflow from Rust to CI
- add name to build-test step of Build & Test
refactor(ci)!: add new `fmt` and `clippy` jobs

- adds two new jobs for `fmt` and `clippy`.
- use `dtolnay/rust-toolchain@v1` instead of `actions-rs/toolchain@v1`

chore(rustfmt): add `.rustfmt.toml` and use `nighly on fmt CI step
- bumps the `edition` on `Cargo.toml` to `2021.
- add `.clippy.toml` with `msrv=1.63.0` file.
Copy link
Contributor

@ValuedMammal ValuedMammal left a comment

Choose a reason for hiding this comment

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

ACK 1a4d5cf

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 1a4d5cf

@notmandatory notmandatory merged commit e4a23af into bitcoindevkit:master Sep 3, 2024
25 checks passed
@oleonardolima oleonardolima deleted the chore/upgrade-and-bump-ci branch September 4, 2024 13:22
notmandatory added a commit that referenced this pull request Sep 25, 2024
…ntation improvements

1103936 chore(style+docs): style and docs retouch (Leonardo Lima)
442789c chore(style): update new methods style (Leonardo Lima)
565d79e refactor(async): add common GET and POST methods (Leonardo Lima)
31dfa4b chore(docs): minor improvements on docstrings (Leonardo Lima)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  It builds on top of #95. Applies minor improvements on some docstrings, and Cargo.toml standard.

  The main change is adding the common HTTP methods for the `AsyncClient`, it removes duplicated code from each Esplora API request, and follows the approach done for `BlockingClient`.

  It makes it easier to extract these methods into an `AsyncEsploraClient` trait (to be done in another PR, initially done here 9cbc387), which the user can implement with any HTTP client of its choice. Also, makes it simpler to rebase and update the `AsyncAnonymizedClient` from #67.

  <!-- Describe the purpose of this PR, what's being adding and/or fixed -->

  ### Notes to the reviewers

  It has some commits from #95, as it builds on top of it and should be merged afterward. Please let me know what you think about the proposed changes and approach.

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Changelog notice

  - Applies minor improvements on documentation.
  - Add common `get_response` and `post_request` methods to `AsyncClient`, previously duplicated through the esplora API calls. It follows the approach done for `BlockinClient`.

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  ValuedMammal:
    ACK 1103936
  notmandatory:
    ACK 1103936

Tree-SHA512: 5579e0cba105f553782e419a16c26fc75b38a2ab8ee523f5ce5e2b9a4560503ef7f81faac546429851802efb66cf125cb36b49f20f088969a6ad580e644d43e5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants