This repository has been archived by the owner on Nov 30, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 52
Improve feature coverage in CI test suite #147
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
20e4c39
Remove duplicate test
tcharding f6603c2
Run tests without std enabled
tcharding f368608
Enable running tests without an allocator
tcharding 41807c0
Add feature documentation to manifest
tcharding 5dc3fd4
Add TODO about features to test once we bump MSRV
tcharding File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In af992d7:
Did you intend to comment out this line? From your commit message it sounds like you do intend to test std and schemars.
We definitely need schemars to be tested in CI somehow. Historically we have had breakage from this dependency that went undetected by CI. (This is why, incidentally, I worte all my own independent CI scripts, and also why schemars is not a dependency in any other rust-bitcoin crate.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shit, I've been a bit sloppy lately fixing commit messages when re-basing. I'm still struggling big time with Rust 1.29, I have some shell functions to build with 1.29 but I often forget to use them because they are not well tied in with my editor ... so I'm constantly re-basing and getting punished by CI. You might have noticed all the force pushes in my PRs :) I'm being lazy, we are so close to bumping to 1.41
We have
extended_tests
which testbitcoin_hashes
withserde
,schemars
, andstd
enabled. Theschemars
feature doesn't build on its own at the momentcargo check --no-default-features --features=schemars
fails. I didn't investigate why I just added this comment to remind us.I added this commented out line with the MSRV comment since I've seen that done in a few places and have started doing it.
Now as I write this message I think it would be better to add all these to the edition 2018 tracking issue instead of leaving it in the code.Oh that's right, the tracking issue is not on this repo its on rust-bitcoin.Will fix the commit log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, good observation that we'll be bumping to 1.41.0 soon, and at that point we can put schemars into the normal CI pipeline.