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.

Sign up for GitHub

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

Jump to bottom

Log macros and verbosity level change for commit messages #558

Merged
merged 3 commits into from
Mar 8, 2017
Merged

Log macros and verbosity level change for commit messages #558

merged 3 commits into from
Mar 8, 2017

Conversation

arichiardi
Copy link
Contributor

As per #557.

arichiardi
arichiardi commented Jan 3, 2017
View reviewed changes
boot/pod/src/boot/util.clj
[verbosity fmt & args]
`(when (>= @*verbosity* ~verbosity)
(binding [*out* *err*]
(print (ansi/bold-cyan (format ~fmt ~@args))))))
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, maybe the color part can be improved here (haven't noticed that immediately, maybe cause it's late 😄)

@arichiardi arichiardi changed the title Increase min verbosity for Commit messages WIP Increase min verbosity for Commit messages Jan 3, 2017
@arichiardi arichiardi changed the title WIP Increase min verbosity for Commit messages Increase min verbosity for Commit messages - log macros Jan 6, 2017
@arichiardi arichiardi force-pushed the commit-verbosity-inc branch 2 times, most recently from 8d222fb to 7cdcbf8 Compare January 6, 2017 06:40
@arichiardi
Copy link
Contributor Author

arichiardi commented Jan 6, 2017 •
edited
Loading

My rigorous testing 😄

2017-01-06-094614_115x206_scrot

  (boot.util/trace "Fn %s\n" 1)
  (boot.util/dbug "Fn %s\n" 1)
  (boot.util/info "Fn %s\n" 1)
  (boot.util/warn "Fn %s\n" 1)
  (boot.util/fail "Fn %s\n" 1)
  (boot.util/trace* "Macro %s\n" 1)
  (boot.util/dbug* "Macro %s\n" 1)
  (boot.util/info* "Macro %s\n" 1)
  (boot.util/warn* "Macro %s\n" 1)
  (boot.util/fail* "Macro %s\n" 1)

@arichiardi arichiardi changed the title Increase min verbosity for Commit messages - log macros Log macros and verbosity level change for commit messages Jan 6, 2017
@micha
Copy link
Contributor

micha commented Jan 6, 2017

Looks good, thanks @arichiardi! Can we make the trace level output cyan (maybe not bold)? That would help differentiate it from stdout.

@arichiardi
Copy link
Contributor Author

We sure can 😀

@arichiardi
Copy link
Contributor Author

@micha @alandipert Done ;)

@arichiardi
Copy link
Contributor Author

Another idea, should we make Aether messages trace now?

rfkm
rfkm reviewed Jan 12, 2017
View reviewed changes
boot/aether/src/boot/aether.clj
@@ -68,7 +68,7 @@

(defn transfer-listener
[info]
(util/dbug "Aether: %s\n" (with-out-str (pprint/pprint info)))
(util/trace* "Aether: %s\n" (with-out-str (pprint/pprint info)))
Copy link

Choose a reason for hiding this comment

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

FYI:
This should fix the bad network throughput. (ref. #565)
In my environment, it takes about 10 minutes to fetch 20MB jar file with dbug, while it takes only 20 seconds with dbug*. I tried to make a PR by myself but I will wait for this PR to be merged.
BTW, it seems that this code was introduced by this conflict resolution commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, macro version is definitively a good call here, else the pprint is called for the info even if dbug prints are disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rfkm can you by any chance clone and confirm that this solves?

Copy link

Choose a reason for hiding this comment

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

@arichiardi I've just tested with this branch and confirmed the trace* version is up to 100 times faster than the dbug version 👍

This was referenced Jan 12, 2017
Closed
Closed
@alandipert alandipert merged commit 562691b into boot-clj:master Mar 8, 2017
@micha micha removed the ready label Mar 8, 2017
@arichiardi arichiardi deleted the commit-verbosity-inc branch March 8, 2017 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfkm rfkm rfkm left review comments

@Deraen Deraen Deraen left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@arichiardi @micha @Deraen @rfkm @alandipert