Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filebrowser dev #184

Merged
merged 38 commits into from
May 11, 2016
Merged

Filebrowser dev #184

merged 38 commits into from
May 11, 2016

Conversation

javo
Copy link
Contributor

@javo javo commented Jan 7, 2016

Fixes for fileretrieval including depth and maxBuffer


//////////////////////////////////////////
// Prey JS FileRetrieval
// (c) 2015 - Fork Ltd.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sign as 2016 - Prey, Inc.


//////////////////////////////////////////
// Prey JS FileRetrieval
// Sign as 2016 - Prey, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace Sign as 2016 - Prey, Inc. with (C) 2016 Prey, Inc.

}
storage.set(NAMESPACE + id, opts);
}
else
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be neccesary

files = require('./files_storage'),
EventEmitter = require('events').EventEmitter;

var common = require('./../../common'),
Copy link
Contributor

Choose a reason for hiding this comment

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

all require calls should be in one var group and non require calls should be in a different var group

// First group
var a = require('a'),
  b = require('b');

// Second group
var something_with_a = a.foo(),
  something_with_b = b.bar();

@raliste
Copy link
Contributor

raliste commented May 11, 2016

👏 Congrats on your first really-big PR!

As the feature is still in beta, we can proceed with merging, BUT, take note of the following requirements for the next minor:

  • Missing unit tests for FR libs
  • Missing integration test with the upload server
  • Add a DESIGN.md doc explaining how the upload protocol works, so that the open source community can implement their own servers if required (the integration test should also help)

@raliste raliste merged commit 4f94144 into master May 11, 2016
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