Organize feature separation for ergonomics#501
Merged
DanGould merged 6 commits intopayjoin:masterfrom Jan 20, 2025
Merged
Conversation
Collaborator
Pull Request Test Coverage Report for Build 12874561764Details
💛 - Coveralls |
These features were separated when v1 was first-class, but is not relevant with v2 being first-class, since the server hosting constraint was removed by the design of v2. All downstream implementations I am aware of are interested in implementing both send and receive features. Close payjoin#393
This way you can enable just the one you're interested in without the additional noise of details you will not use See payjoin#392
This marks Payjoin v2 (BIP 77) is a first-class citizen of the crate.
spacebear21
reviewed
Jan 20, 2025
contrib/coverage.sh
Outdated
| # https://github.com/taiki-e/cargo-llvm-cov?tab=readme-ov-file#merge-coverages-generated-under-different-test-conditions | ||
| cargo llvm-cov clean --workspace # remove artifacts that may affect the coverage results | ||
| cargo llvm-cov --no-report # v1 configuration | ||
| cargo llvm-cov --no-report --features=v1 |
Collaborator
There was a problem hiding this comment.
Might as well add _danger-local-https here now since it's required for some v1 tests (e.g. e2e tests), and partially address #499
contrib/lint.sh
Outdated
| set -e | ||
|
|
||
| cargo clippy --all-targets --keep-going -- -D warnings # v1 configuration | ||
| cargo clippy --all-targets --keep-going --features=v1 -- -D warnings |
Without this configuration some checks would be ignored. Close payjoin#499
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
sendreceivefeature separation #393receive::v2,send::v2submodule into additivecrate::v2andcrate::v1submodules features #392 hard with an explicitv1featurepayjoin/v2a default featurepayjoin-cliv1 coverage #499 (Configure v1 checks to use _danger-local-https)In making these changes I also considered an organizational structure that looks more like this:
payjoin::receive::{InputPair, Error...}payjoin::v1::receive,payjoin::v2::receiveBut I think that's more trouble than it's worth, and also breaks the hierarchy that's most effective for producing and maintaining this software, so I'm inclined to leave it as is, leaving
send,receivemodules abstraction with version-specific submodule implementations.This makes it easier for us to effectuate #500