closes #421#433
Conversation
|
@ogazboiz please review |
|
hey, main was failing CI from broken auth imports + frontend parse errors. fixed and pushed to main now. please rebase to pick up the fixes: git fetch upstream
git rebase upstream/main
git push --force-with-leaseif there's a conflict, resolve it locally and we'll review once CI is green. |
|
ok im on it |
8594f72 to
805ce6a
Compare
|
@ogazboiz CI failure fixed |
ogazboiz
left a comment
There was a problem hiding this comment.
hey @dubemoyibe-star, the contract change looks great — stream_count is exactly what #421 needed and the test covers the cases well.
a few unrelated changes need to be removed before this can merge:
-
backend/src/controllers/stream/cancel.ts— has weird formatting (mixed indentation) and treatsreq.params.streamIdas if it could be an array, but Express params are always strings. this change is incorrect and out of scope. please remove. -
backend/src/services/sorobanService.ts— adds.build()to assembleTransaction. could be a real fix but it's out of scope for this PR. either drop it or split into a separate PR. -
backend/src/types/auth.types.ts— makingidoptional reduces type safety unnecessarily. please remove unless there's a specific reason.
once the diff is just contracts/stream_contract/src/lib.rs + test.rs (the core feature), this is good to merge.
|
@ogazboiz Chief |
|
hey @dubemoyibe-star, glad CI passes now, but the scope feedback hasn't been addressed:
CI passing is not the same as PR being ready. issue #421 only asked for please run: git checkout origin/main -- backend/src/controllers/stream/cancel.ts backend/src/services/sorobanService.ts backend/src/types/auth.types.ts
git commit -am "chore: remove unrelated backend changes"
git pushonce the diff is just the two |
Description
Type of Change
Related Issues
Closes #421
Changes Made
Testing
Test Coverage
Test Steps
Breaking Changes
Breaking Changes:
Migration Guide:
Screenshots/Demo
Checklist
Additional Notes