LBRYCommunity/lbry-redux
1
0
Fork 0
Code Issues 5 Pull requests 6 Packages Projects Releases Wiki Activity

MERGE AFTER 34 RELEASE - enables publish for desktop - flow skipped WIP #162

Merged
jessopb merged 4 commits from reduxPublish into master 2019-07-29 20:43:01 +02:00
Conversation 0 Commits 4 Files changed 8 +257 -328
jessopb commented 2019-07-03 16:56:20 +02:00 (Migrated from github.com)
Copy link

doPublish selects redux state for publish
re: comments here https://github.com/lbryio/lbry-redux/pull/158

@akinwale How bad are the compatibility issues?

doPublish selects redux state for publish re: comments here https://github.com/lbryio/lbry-redux/pull/158 @akinwale How bad are the compatibility issues?
akinwale (Migrated from github.com) reviewed 2019-07-05 14:27:50 +02:00
akinwale (Migrated from github.com) left a comment
Copy link

Looks good so far. Just a couple of things to address.

Looks good so far. Just a couple of things to address.
src/redux/actions/publish.js
@ -315,47 +350,14 @@ export const doPublish = (params: PublishParams) => (dispatch: Dispatch, getStat
// The sdk will figure it out
if (filePath) publishPayload.file_path = filePath;
akinwale (Migrated from github.com) commented 2019-07-05 14:24:08 +02:00
Copy link

Probably want to remove this before merging.

Probably want to remove this before merging.
src/redux/selectors/publish.js
akinwale (Migrated from github.com) commented 2019-07-05 14:27:24 +02:00
Copy link

What about the rest of the selectors, like selectTakeOverAmount? That may still be useful.

What about the rest of the selectors, like `selectTakeOverAmount`? That may still be useful.
neb-b (Migrated from github.com) requested changes 2019-07-09 17:48:54 +02:00
neb-b (Migrated from github.com) left a comment
Copy link

A couple of comments

A couple of comments
flow-typed/Publish.js
neb-b (Migrated from github.com) commented 2019-07-09 17:48:44 +02:00
Copy link

We can just delete this

We can just delete this
src/redux/actions/publish.js
neb-b (Migrated from github.com) commented 2019-07-09 17:45:18 +02:00
Copy link

lbry-redux doesn't know about these comments, so they won't do anything.

lbry-redux doesn't know about these comments, so they won't do anything.
src/redux/actions/publish.js
neb-b (Migrated from github.com) commented 2019-07-09 17:48:34 +02:00
Copy link

After looking more I think these can be removed anyway

After looking more I think these can be removed anyway
src/redux/actions/publish.js
@ -230,2 +249,4 @@
const myChannels = selectMyChannelClaims(state);
const myClaims = selectMyClaimsWithoutChannels(state);
// get redux publish form
const publishData = selectPublishFormValues(state);
neb-b (Migrated from github.com) commented 2019-07-09 17:46:24 +02:00
Copy link

needed?

needed?
src/redux/actions/publish.js
neb-b (Migrated from github.com) commented 2019-07-09 17:47:35 +02:00
Copy link

doCheckPendingPublishes should have success callbacks too. That's where the desktop can trigger the notification

`doCheckPendingPublishes` should have success callbacks too. That's where the desktop can trigger the notification
jessopb (Migrated from github.com) reviewed 2019-07-10 16:46:09 +02:00
src/redux/actions/publish.js
jessopb (Migrated from github.com) commented 2019-07-10 16:46:09 +02:00
Copy link

@akinwale @seanyesmunt I thought @akinwale added fsAdapter for android, but desktop will still use 'fs' and 'path'. I pulled this logic into the selectAsset component for channel Edits, but publish still uses it as is?

@akinwale @seanyesmunt I thought @akinwale added fsAdapter for android, but desktop will still use 'fs' and 'path'. I pulled this logic into the selectAsset component for channel Edits, but publish still uses it as is?
jessopb (Migrated from github.com) reviewed 2019-07-10 16:47:27 +02:00
src/redux/actions/publish.js
@ -230,2 +249,4 @@
const myChannels = selectMyChannelClaims(state);
const myClaims = selectMyClaimsWithoutChannels(state);
// get redux publish form
const publishData = selectPublishFormValues(state);
jessopb (Migrated from github.com) commented 2019-07-10 16:47:27 +02:00
Copy link

nope

nope
neb-b (Migrated from github.com) reviewed 2019-07-10 17:33:02 +02:00
src/redux/actions/publish.js
neb-b (Migrated from github.com) commented 2019-07-10 17:33:02 +02:00
Copy link

I think fs can stay

I think `fs` can stay
neb-b (Migrated from github.com) approved these changes 2019-07-15 19:56:16 +02:00
neb-b (Migrated from github.com) left a comment
Copy link

Looks great!

Looks great!
Sign in to join this conversation.
Reviewers
No reviewers
neb-b
Labels
Clear labels   
area: devops

   
area: discovery

   
area: docs

   
area: livestream

   
area: proposal

Work is part of a proposal

  
consider soon

Discuss this issue at the next planning meeting, then remove this label

  
dependencies

Pull requests that update a dependency file

  
Epic

   
good first issue

Good for newcomers

  
hacktoberfest

Welcome to Hacktoberfest

  
help wanted

Extra attention is needed

  
icebox

Long-term storage

  
Invalid

   
level: 1

No knowledge of the existing code required

  
level: 2

Some knowledge of the existing code is recommended

  
level: 3

Significant knowledge of the existing code is recommended

  
level: 4

Intimate knowledge of the existing code is recommended

  
needs: exploration

Solution unclear, needs research

  
needs: grooming

Issue needs to be groomed before work can start

  
needs: priority

Priority level needs to be defined

  
needs: repro

Issue needs step-by-step instructions on how to reproduce in latest code

  
needs: tech design

Needs technical design signoff before implementation can begin

  
on hold

Temporarily paused

  
priority: blocker

Issue is blocking release, do ASAP

  
priority: high

Work needs to be moved into sprint ASAP

  
priority: low

Work should be done but can stay in the backlog for now

  
priority: medium

Work needs to be done within 2-3 sprints

  
resilience

   
Tom's Wishlist

   
type: bug

Existing functionality is wrong or broken

  
type: discussion

A conversation. No specific changes requested

  
type: improvement

Existing (or partially existing) functionality needs to be changed

  
type: new feature

New functionality that does not exist yet

  
type: refactor

Minimal user-visible changes, but significant internal work

  
type: task

Either work that's not related to the code, or a small chore that does not fit into other categories

  
type: testing

Solution needs additional user testing

  
unplanned

Work that was not planned into the spirnt, took priority over planned work

No labels
area: devops
area: discovery
area: docs
area: livestream
area: proposal
consider soon
dependencies
Epic
good first issue
hacktoberfest
help wanted
icebox
Invalid
level: 1
level: 2
level: 3
level: 4
needs: exploration
needs: grooming
needs: priority
needs: repro
needs: tech design
on hold
priority: blocker
priority: high
priority: low
priority: medium
resilience
Tom's Wishlist
type: bug
type: discussion
type: improvement
type: new feature
type: refactor
type: task
type: testing
unplanned
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-redux#162
No description provided.
Delete branch "reduxPublish"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?