Skip to content

Conversation

flovilmart
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Sep 2, 2017

Codecov Report

Merging #4131 into master will increase coverage by 1.24%.
The diff coverage is 94.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4131      +/-   ##
==========================================
+ Coverage   90.84%   92.08%   +1.24%     
==========================================
  Files         116      116              
  Lines        8028     7960      -68     
==========================================
+ Hits         7293     7330      +37     
+ Misses        735      630     -105
Impacted Files Coverage Δ
src/triggers.js 93.75% <ø> (+2.84%) ⬆️
src/Adapters/Storage/Mongo/MongoCollection.js 97.5% <ø> (+2.37%) ⬆️
src/Routers/RolesRouter.js 87.5% <0%> (+46.32%) ⬆️
src/Routers/InstallationsRouter.js 100% <100%> (+23.33%) ⬆️
src/Routers/ClassesRouter.js 93.33% <100%> (+0.27%) ⬆️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.49% <100%> (+0.02%) ⬆️
src/Push/PushWorker.js 94.91% <100%> (+1.36%) ⬆️
src/Routers/UsersRouter.js 91.05% <100%> (+0.68%) ⬆️
src/Routers/SessionsRouter.js 93.75% <100%> (+3.5%) ⬆️
src/Routers/AudiencesRouter.js 100% <100%> (+8.57%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3079270...b988c2d. Read the comment docs.

@flovilmart flovilmart force-pushed the coverage-nits branch 2 times, most recently from 7c3e67e to ff25ba1 Compare September 2, 2017 03:32
@flovilmart flovilmart requested a review from acinader September 5, 2017 00:46
@flovilmart flovilmart changed the title WIP: Finding areas that are untested and need love Finding areas that are untested and need love Sep 5, 2017
@flovilmart
Copy link
Contributor Author

flovilmart commented Sep 5, 2017

@acinader I'm done with that one! :)

  • removes boat load of dead code
  • small harmless refactors
  • tests and fixes for the range file requests.

Copy link
Contributor

@acinader acinader left a comment

Choose a reason for hiding this comment

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

niiiice.

src/RestWrite.js Outdated
@@ -1011,9 +1011,19 @@ RestWrite.prototype.runDatabaseOperation = function() {
if (this.className !== '_User' || error.code !== Parse.Error.DUPLICATE_VALUE) {
throw error;
}

// Quick check, if we were able to infer the duplicated field name
if (error && error.userInfo && error.userInfo.duplicated_field == 'username') {
Copy link
Contributor

Choose a reason for hiding this comment

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

string.equals or ===

}

function getRange(req) {
const parts = req.get('Range').replace(/bytes=/, "").split("-");
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer ' to ". stay consistent

}

// handleFileStream is licenced under Creative Commons Attribution 4.0 International License (https://creativecommons.org/licenses/by/4.0/).
// Author: LEROIB at weightingformypizza (https://weightingformypizza.wordpress.com/2015/06/24/stream-html5-media-content-like-video-audio-from-mongodb-using-express-and-gridstore/).
function handleFileStream(stream, req, res, contentType) {
var buffer_size = 1024 * 1024;//1024Kb
const buffer_size = 1024 * 1024; //1024Kb
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 :).

end = start + (buffer_size);
}
chunksize = (end - start) + 1;
// No start profided, we're reading backwards
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in the comment: profided

//totalbytes = 0;
// in case of small slices, all values will be good at that point
// we've written enough, end...
if (totalBytesWritten >= contentLength) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm no expert at js io. is there a try / catch / finally idiom?

maybe something like: https://stackoverflow.com/questions/4092914/java-try-catch-finally-best-practices-while-acquiring-closing-resources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhm, I'm not sure, I kept the original code there, as it work working as expected, the changes I put in were just because the code was a duplicate. if it throws, express will catch it :)

@flovilmart flovilmart merged commit 139b9e1 into master Sep 5, 2017
@flovilmart flovilmart deleted the coverage-nits branch September 5, 2017 21:51
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.

2 participants