-
Notifications
You must be signed in to change notification settings - Fork 21
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
refactor: filecoin api services events and tests #974
refactor: filecoin api services events and tests #974
Conversation
1ec5761
to
2d860d4
Compare
ee803fa
to
3289cc1
Compare
c78b7e3
to
def20a3
Compare
def20a3
to
127b110
Compare
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.
Wow what a PR. In general this is great. My biggest concern is wrapping errors and losing valuable context.
export type FilecoinAcceptFailure = | ||
| InvalidContentPiece | ||
| ProofNotFound | ||
| Ucanto.Failure |
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.
I seem to recall gozala saying we shouldn't add generic Failure
to these unions...(there are more instances of this).
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.
well this looks to be everywhere... Even in the recent capabilities like RateLimit, Customer, ...
If I remove it, there are a lot of typescript failures, so I will keep it and we can consider doing this removal across the board
* @param {API.Test<AggregatorApi.TestEventsContext>} testFn | ||
* @param {(context: AggregatorApi.TestEventsContext) => Promise<AggregatorApi.TestEventsContext>} mockContextFunction | ||
*/ | ||
function wichMockableContext(testFn, mockContextFunction) { |
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.
A helper?
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.
I could not get this to work with the chain of type inference if I put this with templates to try to make it generic with all the contexts being inferred by caller. So keeping it for now to not spend more time
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.
Can fix the typo though?
function wichMockableContext(testFn, mockContextFunction) { | |
function withMockableContext(testFn, mockContextFunction) { |
provider: 'f099' | ||
} | ||
} | ||
12_345: { |
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.
Hmm...why?
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.
linter wants this...
3bab254
into
refactor/filecoin-client
Fixes aggregate/accept receipt: 1. inclusion proof was wrongly here. It should only be in `piece/accept` because it is piece specific proof within an aggregate 2. added `aggregate` in receipt to be coherent with other receipts for `*/accept` Fixes filecoin/accept receipt: 1. added `piece` and `aggregate` in receipt to be coherent with other receipts for `*/accept` 2. swaped `aux` to be a property per storacha/w3up#974 (comment)
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
Implements `filecoin-api` services, events and tests all over the place. BREAKING CHANGE: see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md
🤖 I have created a release *beep* *boop* --- ## [11.0.0](capabilities-v10.2.0...capabilities-v11.0.0) (2023-10-24) ### ⚠ BREAKING CHANGES * see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md * filecoin client to use new capabilities * filecoin capabilities ### Bug Fixes * add missing ContentNotFound definition for filecoin offer failure ([c0b97bf](c0b97bf)) * add missing filecoin submit success and failure types ([c0b97bf](c0b97bf)) * client tests ([b0d9c3f](b0d9c3f)) * type errors ([c0b97bf](c0b97bf)) * upgrade ucanto in filecoin api ([c95fb54](c95fb54)) ### Code Refactoring * filecoin api services events and tests ([#974](#974)) ([953537b](953537b)) * filecoin capabilities ([c0b97bf](c0b97bf)) * filecoin client to use new capabilities ([b0d9c3f](b0d9c3f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [2.0.0](filecoin-client-v1.4.0...filecoin-client-v2.0.0) (2023-10-24) ### ⚠ BREAKING CHANGES * see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md * filecoin client to use new capabilities ### Bug Fixes * client tests ([b0d9c3f](b0d9c3f)) * upgrade ucanto in filecoin api ([c95fb54](c95fb54)) ### Code Refactoring * filecoin api services events and tests ([#974](#974)) ([953537b](953537b)) * filecoin client to use new capabilities ([b0d9c3f](b0d9c3f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Vasco Santos <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [2.0.0](filecoin-api-v1.4.4...filecoin-api-v2.0.0) (2023-10-24) ### ⚠ BREAKING CHANGES * see latest specs https://github.com/web3-storage/specs/blob/cbdb706f18567900c5c24d7fb16ccbaf93d0d023/w3-filecoin.md ### Features * upgrade to ucanto@9 ([#951](#951)) ([d72faf1](d72faf1)) ### Bug Fixes * upgrade ucanto in filecoin api ([c95fb54](c95fb54)) ### Code Refactoring * filecoin api services events and tests ([#974](#974)) ([953537b](953537b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Vasco Santos <[email protected]>
Implements
filecoin-api
services, events and tests all over the place.Note that some small follow ups are still needed and will be added as follow up:
piece/accept