refactor doPurchaseUri #149

2019-05-16T08:18:31+02:00

akinwale commented

2019-05-16 08:18:31 +02:00

(Migrated from github.com)

No description provided.

neb-b (Migrated from github.com) requested changes 2019-05-16 18:11:41 +02:00

neb-b (Migrated from github.com) left a comment

Looks good. My biggest comment is to add flow coverage to these files.

src/redux/actions/file.js

neb-b (Migrated from github.com) commented

2019-05-16 17:50:50 +02:00

This should be an error toast

neb-b (Migrated from github.com) commented

2019-05-16 17:58:34 +02:00

streamInfo: GetResponse

`streamInfo: GetResponse`

src/redux/actions/file.js

					
				@ -0,0 +25,4 @@

				        if (timeout) {

				          dispatch({

				            type: ACTIONS.LOADING_FILE_FAILED,

neb-b (Migrated from github.com) commented

2019-05-16 18:11:16 +02:00

LOADING_FILE_X is everywhere that PURCHASE_URI_X is. Do we need both?

`LOADING_FILE_X` is everywhere that `PURCHASE_URI_X` is. Do we need both?

src/redux/reducers/file.js

					
				@ -0,0 +2,4 @@

				import * as ACTIONS from 'constants/action_types';

				const reducers = {};

				const defaultState = {

neb-b (Migrated from github.com) commented

2019-05-16 18:00:30 +02:00

Please create a type for FileState

Please create a type for `FileState`

kauffj (Migrated from github.com) reviewed 2019-05-20 21:54:18 +02:00

src/redux/actions/file.js

kauffj (Migrated from github.com) commented

2019-05-20 20:58:52 +02:00

Maybe doGet or doFileGet? Since our naming for actions generally matches or includes the SDK method names

Maybe `doGet` or `doFileGet`? Since our naming for actions generally matches or includes the SDK method names

kauffj (Migrated from github.com) commented

2019-05-20 21:00:05 +02:00

If this error surfaces to users, it should be written more clearly (Maybe just "Already fetching ${uri}")

neb-b (Migrated from github.com) reviewed 2019-05-21 21:06:48 +02:00

src/redux/reducers/file.js

neb-b (Migrated from github.com) commented

2019-05-21 21:06:47 +02:00

This returns FileState

This returns `FileState`

Sign in to join this conversation.

No reviewers

neb-b

No milestone

No project

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#149