Skip to content

Channel payouts: Mappings, CLI, Integration-tests, TS lib#4334

Merged
mnaamani merged 70 commits intoJoystream:ephesusfrom
zeeshanakram3:channel_payouts
Nov 13, 2022
Merged

Channel payouts: Mappings, CLI, Integration-tests, TS lib#4334
mnaamani merged 70 commits intoJoystream:ephesusfrom
zeeshanakram3:channel_payouts

Conversation

@zeeshanakram3
Copy link
Copy Markdown
Contributor

supersedes #3879 & #3182

Tracking issue #3880

zeeshanakram3 and others added 30 commits February 7, 2022 11:42
@mnaamani mnaamani self-requested a review November 8, 2022 11:02
@mnaamani
Copy link
Copy Markdown
Member

mnaamani commented Nov 9, 2022

@zeeshanakram3 as discussed I'm updating ephesus branch here #4430 Once that is merged you can re-target this PR to ephesus

Copy link
Copy Markdown
Collaborator

@kdembler kdembler left a comment

Choose a reason for hiding this comment

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

Thanks for making the package hybrid! We verified that it works properly with Atlas now, without the need for patching which is great. LGTM from me and @drillprop

There's some failing test though, not sure what that is about

@mnaamani
Copy link
Copy Markdown
Member

There's some failing test though, not sure what that is about

I'll re-run them, but a proper fix for this occasional failure is in carthage branch now

@kdembler
Copy link
Copy Markdown
Collaborator

btw, @mnaamani @zeeshanakram3, Atlas team will need those new libraries published so that we can consume them and start developing. That's new version of @joystream/metadata-protobuf and new library @joystream/js

@mnaamani mnaamani changed the base branch from carthage to ephesus November 11, 2022 10:01
@mnaamani
Copy link
Copy Markdown
Member

@zeeshanakram3 I retargeted the PR to ephesus branch. Perhaps you can rebase PR on ephesus just to be sure its all good.

Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Please update PR from ephesus branch and bump metadata-protobuf package version to 3.0.0

{
"name": "@joystream/metadata-protobuf",
"version": "2.5.0",
"version": "2.6.0",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets bump this to v3.0.0 to allow for version updates on carthage/mainnet in the 2.x.x range.
And of course update the dependent packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. btw moving forward, wouldn't it be a better solution to have something like 2.6.0-beta for pre-production /testing releases instead of reserving some range for a specific release, i.e., 2.x.x range for carthage/mainnet. This way, devs can be more open to following semantic versioning guidelines properly, i.e., bumping major versions too in case of breaking changes/new features

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ok makes sense.

Copy link
Copy Markdown
Contributor Author

@zeeshanakram3 zeeshanakram3 Nov 13, 2022

Choose a reason for hiding this comment

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

Sorry, but I haven't made the version bump change yet; I was just confirming if it's fine to do it this way before making the change. So will you reopen this PR, Or should I create a separate PR including the version change, i.e., setting @joystream/metadata-protobuf version to 2.6.0-beta
@mnaamani

@mnaamani mnaamani merged commit 46a2b4e into Joystream:ephesus Nov 13, 2022
@Lezek123 Lezek123 mentioned this pull request Jan 19, 2023
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants