schedule bugfix for lastSchedule, airbnb-ish, logging, cleanup #170
Conversation
|
λ/10 |
modules/schedule/index.js
Outdated
|
|
||
| reply("Removed schedule by \"" + s.blame + "\" which runs \"" + s.command + "\"" ); | ||
| reply(`Removed schedule by "${s.blame}" which runs "${s.command}"`); |
There was a problem hiding this comment.
I think this tense is correct here. The flow is:
!schedule add "every weekday" "Hey!"
...
Hey!
...
Hey!
!schedule remove hash
Removed schedule by "brhoades" which runs "Hey!"
There was a problem hiding this comment.
I think it'd be correct to say either Removing ... which runs ... or Removed ... which ran ....
There was a problem hiding this comment.
For some reason I didn't read the first part of the string? It should be removing... changed.
There was a problem hiding this comment.
I guess the most correct might be "which would have run".
Anyways, you're right, fine as is.
modules/schedule/index.js
Outdated
| next = next.map(sched => new Moment(sched)); | ||
|
|
||
| for (let i = 1; i <= numSamples; i++) { | ||
| totalSeconds += parseInt(getDifference(next[i + 1], next[i]).format('s'), 10); |
There was a problem hiding this comment.
Number.parseInt is mildly preferred these days and behaviourally identical.
modules/schedule/index.js
Outdated
| const writeSchedule = (data) => { | ||
| bot.writeDataFile('later.json', JSON.stringify(data), (err) => { | ||
| if (err) { | ||
| log.debug(`Error writing command file: ${err}`); |
modules/schedule/index.js
Outdated
|
|
||
| const sortSchedules = scheds => ( | ||
| Object.values(scheds).sort((a, b) => { | ||
| if (a.created > b.created) { |
There was a problem hiding this comment.
Since you can subtract moments and get a number, this can be simplified to:
sort((a, b) => a.created - b.created)
modules/schedule/index.js
Outdated
| var m = "Provided schedule query doesn't parse:\n"; | ||
| var offset = 0; | ||
| if (typeof(s) === 'number' || s.error >= 0) { | ||
| let m = 'Provided schedule query doesn\'t parse:\n'; |
There was a problem hiding this comment.
backtick quotes to avoid the escaping thing?
There was a problem hiding this comment.
eslint with airbnb throws a fit if you use backticks without any interpolation.
There was a problem hiding this comment.
Because of avoidEscape: true here https://github.com/airbnb/javascript/blob/eslint-config-airbnb-v16.1.0/packages/eslint-config-airbnb-base/rules/style.js#L391-L392, I think that you can actually use double-quotes here and still be on airbnb's good side.
brhoades
force-pushed
the
schedule-update
branch
from
2314698
to
2177ad8
Compare
|
Rebased, added schedule.js to lint script. |
Updated eslint targets, cleaned up schedule more. Updates from PR.
brhoades
force-pushed
the
schedule-update
branch
from
2177ad8
to
c42450b
Compare
| let minimumCreationDelay = 0; | ||
| let minimumInterval = 60; | ||
| let noCommands = false; | ||
| let digestLength = 4; |
There was a problem hiding this comment.
curious; why the digestLength change?
There was a problem hiding this comment.
Back of the envelope math. I figured It's more likely schedule will break down due to performance issues before the threshold for a 1% collision chance.
Really: I got sick of testing and having to type out 8 letters ids to remove a schedule.
|
(To be clearer with what I meant, feel free to self-merge if you're happy with it @brhoades) |
…wobscale#170) Updated eslint targets, cleaned up schedule more. Updates from PR.
Bugfix for lastSchedule not caching properly (you could only ever have one schedule).
Now using log (part of #144).
General cleanup, made the file mildly airbnb styled.