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

Print timestamps on "END" events. #1620

Merged
merged 2 commits into from
Sep 16, 2017

Conversation

jacksonrayhamilton
Copy link
Contributor

Previously, it was not clear when work had been completed. This sets clearer expectations for someone monitoring the watcher output as to when they can expect their code to have updated.

Previously, it was not clear when work had been completed.  This sets clearer
expectations for someone monitoring the watcher output as to when they can
expect their code to have updated.
@@ -76,7 +76,10 @@ export default function watch(configFile, configs, command, silent) {
break;

case 'END':
if ( !silent && isTTY ) stderr( `\nwaiting for changes...` );
if ( !silent && isTTY ) {
stderr( `\nfinished at ${new Date()}` );
Copy link
Contributor

Choose a reason for hiding this comment

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

The default Date format is rather ...quaint?

$ node -p "new Date().toString()"
Fri Sep 15 2017 15:13:26 GMT-0700 (PDT)

Why does this have to be across multiple lines, anyway? [YYYY-MM-DD HH:MM:SS] waiting for changes... seems more than sufficient. date-time looks like a good library for the formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the timestamp to the "waiting" line and reformatted it.

Copy link
Contributor

@evocateur evocateur left a comment

Choose a reason for hiding this comment

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

Looks great!

@Rich-Harris Rich-Harris merged commit 004478c into rollup:master Sep 16, 2017
@Rich-Harris
Copy link
Contributor

great idea, thank you!

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