appnexus / sicksync

Don’t accept the available as the preferable. Go the extra mile with extra speed.
Apache License 2.0
68 stars 11 forks source link

feat: disable triggerBigSync #84

Open laggingreflex opened 7 years ago

laggingreflex commented 7 years ago

Lets you add an option "triggerBigSync": false in project conf to disable doing a big rsync.

fixes #79

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.04%) to 97.357% when pulling 76b5f7d8306cde871be9cea93583ce6c8dd052fd on laggingreflex:feat/disable-bigSync into 00dc1a8e8ec4a17274340395d12f026127f4f873 on appnexus:master.

laggingreflex commented 7 years ago

I've updated it to be a --disable-rsync cli arg and disableRsync as project config

I've also update the --no-delete cli arg I added earlier to be --disable-deletion cli arg and disableDeleteion project arg

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 96.957% when pulling bbf9754e21c592c1efa5e2eaacb8fc88252af8fa on laggingreflex:feat/disable-bigSync into 00dc1a8e8ec4a17274340395d12f026127f4f873 on appnexus:master.

laggingreflex commented 7 years ago

I'm currently testing it out and having some issues.

Don't merge it yet, I'll let you know when. Sorry for hasty commits.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 96.957% when pulling 5e1635b4844b8c70c216a392fa8a5256d11f9730 on laggingreflex:feat/disable-bigSync into 00dc1a8e8ec4a17274340395d12f026127f4f873 on appnexus:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.4%) to 96.97% when pulling b861ede145e5e98d1e3083703df84d1b979f5348 on laggingreflex:feat/disable-bigSync into 00dc1a8e8ec4a17274340395d12f026127f4f873 on appnexus:master.

laggingreflex commented 7 years ago

I was forgetting about a particular callback which was causing the issue. Wrapping triggerBigSync in an if just like that was not a great idea:

if(..) {
  triggerBigSync(projectConf, { debug: config.debug }, () => {
    localLog(text.SYNC_ON_LARGE_CHANGE_DONE);
    fsHelper.watch();
  });
}

There's a callback there which wouldn't get called.

I've used async/await now (added a babel-preset-stage-0) with which it could be solved and simplified:

if (...) {
  await triggerBigSync(projectConf, { debug: config.debug });
}
localLog(text.SYNC_ON_LARGE_CHANGE_DONE);
fsHelper.watch();

Although it alters the API a bit (triggerBigSync now returns a promise) so some tests might be failing. I'll see if I can work on those later in the week. Feel free to review now.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 96.97% when pulling 0a7dffa0a88af0cca39e2778d9e1eb0e3daec62d on laggingreflex:feat/disable-bigSync into af55b54a2b9ad1131a784bf67e3860d4b067a844 on appnexus:master.

joelgriffith commented 7 years ago

This looks great, we'll see what the CI finds test wise.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 96.97% when pulling 0a7dffa0a88af0cca39e2778d9e1eb0e3daec62d on laggingreflex:feat/disable-bigSync into af55b54a2b9ad1131a784bf67e3860d4b067a844 on appnexus:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 96.97% when pulling 6165fbf3032e546b50cb8f1c84919f9951f13ff7 on laggingreflex:feat/disable-bigSync into af55b54a2b9ad1131a784bf67e3860d4b067a844 on appnexus:master.

joelgriffith commented 7 years ago

I just pushed some updates to get tests running (properly) again, sorry for the thrash. I can update your PR to resolve the conflicts here on Friday.