Skip to content

Add support read hook and implement webAPI based read hook#82

Closed
hellais wants to merge 7 commits intoflowjs:masterfrom
hellais:feature/readHook
Closed

Add support read hook and implement webAPI based read hook#82
hellais wants to merge 7 commits intoflowjs:masterfrom
hellais:feature/readHook

Conversation

@hellais
Copy link
Copy Markdown
Contributor

@hellais hellais commented Feb 25, 2015

Based on discussion and feedback from @AidasK in #42

@hellais
Copy link
Copy Markdown
Contributor Author

hellais commented Feb 25, 2015

I can't run the unittests locally so I am hoping this will kick off the travis unittest.

@AidasK
Copy link
Copy Markdown
Member

AidasK commented Feb 27, 2015

test FlowFile functions should get extension FAILED - fails, because you have deleted getExtension method. It should not be hard to launch tests locally, you only need to install npm. Please ask if you have any serious questions.

@hellais
Copy link
Copy Markdown
Contributor Author

hellais commented Mar 11, 2015

I believe the above error is now fixed. What is the status on getting this merged into upstream?

@evilaliv3
Copy link
Copy Markdown
Member

@AidasK can you provide us a feedback for the possibility to merge it and for a tentative release schedule?

Comment thread src/flow.js
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.

Is read defined?

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.

yes it has the default you suggested:

      read: webAPIFileRead,

that is:

 /**
  * Default read function using the webAPI
  *
  * @function webAPIFileRead(chunk, startByte, endByte, fileType)
  *
  */
 function webAPIFileRead(chunk, startByte, endByte, fileType) {
   var function_name = (chunk.fileObj.file.slice ? 'slice' :
     (chunk.fileObj.file.mozSlice ? 'mozSlice' :
       (chunk.fileObj.file.webkitSlice ? 'webkitSlice' :
         'slice')));
   chunk.readFinished(chunk.fileObj.file[function_name](startByte, endByte, fileType));
 }

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.

read -> var read ?

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.

an other time this is a code pattern (https://github.com/shichuan/javascript-patterns/blob/master/general-patterns/single-var-pattern.html) but given that you do not use it in other parts of the code i will change it.

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.

Sorry, haven't noticed comma in line above. I just try to follow google style guide and to be consistent.

@AidasK
Copy link
Copy Markdown
Member

AidasK commented Jul 17, 2015

Could you also update documentation? It would be great to have an example, how you are using this for compression and encryption. Thanks!

@evilaliv3 evilaliv3 mentioned this pull request Jul 17, 2015
Comment thread src/flow.js
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.

comma might cause a fata error for IE browsers

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.

k, i'm removing it.

evilaliv3 added a commit to evilaliv3/flow.js that referenced this pull request Jul 17, 2015
evilaliv3 added a commit to evilaliv3/flow.js that referenced this pull request Jul 26, 2015
@evilaliv3
Copy link
Copy Markdown
Member

this has been integrated a lot of time ago by means of a different pull request.

@evilaliv3 evilaliv3 closed this Dec 14, 2015
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.

3 participants