Skip to content

Add a short description in README #4

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

Merged
merged 3 commits into from
Oct 21, 2019
Merged

Conversation

tkf
Copy link
Collaborator

@tkf tkf commented Oct 18, 2019

No description provided.

@tkf tkf mentioned this pull request Oct 18, 2019
4 tasks
@pfitzseb
Copy link
Member

The frontend/backend distinction is somewhat problematic imho: ProgressLogging is a frontend for the package author (because it is what they immediately interact with) but a backend for the user (who directly interacts with Juno/ProgressMeterLogging).

Looks good otherwise.

@aviatesk
Copy link

aviatesk commented Oct 18, 2019

yep I had that impression too. Maybe "wrappper", "API interface" or some similar word would fit into this ?

@tkf
Copy link
Collaborator Author

tkf commented Oct 19, 2019

I agree the terminology can be better. Here is some brainstorming:

For progress bar "frontend":

  • progress event source
  • progress data source
  • progress recorder
  • progress definition
  • progress logger [1]
  • progress (event/record/log) emitter

For progress bar "backend":

  • progress monitor
  • progress display [2]
  • progress viewer
  • progress (event/record/log) handler

Notes:

[1] "progress logger" as a frontend kind of makes sense but it's the backend that defines *Logger <: AbstractLogger type (and the handler method handle_message). So it may be confusing.

[2] "progress display" may be slightly confusing as we have the display stack with (somewhat) similar pluggable architecture.

Overall, I think "progress event source" and "progress monitor" sound good.

@codecov-io
Copy link

codecov-io commented Oct 19, 2019

Codecov Report

Merging #4 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master       #4   +/-   ##
=======================================
  Coverage   89.39%   89.39%           
=======================================
  Files           1        1           
  Lines          66       66           
=======================================
  Hits           59       59           
  Misses          7        7
Impacted Files Coverage Δ
src/ProgressLogging.jl 89.39% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b2d3f4...738e04f. Read the comment docs.

@tkf
Copy link
Collaborator Author

tkf commented Oct 19, 2019

Actually, I think we can just say

ProgressLogging.jl is a package for defining progress logs.

I think we can call the loop using @progress etc. a progress log source but probably we don't need this term.

This makes me think that it's probably better to rename ProgressMeterLogging.jl to ConsoleProgressMonitor.jl. What do you think?

@pfitzseb
Copy link
Member

ProgressLogging.jl is a package for defining progress logs.

Yeah, that sounds good.

This makes me think that it's probably better to rename ProgressMeterLogging.jl to ConsoleProgressMonitor.jl. What do you think?

TerminalProgressMonitor.jl maybe? Seems like a good name either way though.

@tkf
Copy link
Collaborator Author

tkf commented Oct 21, 2019

OK, I renamed and re-released tkf/ConsoleProgressMonitor.jl#2 (TerminalProgressMonitor.jl sounds good too but I thought matching with Logging.ConsoleLogger might make sense)

This PR should be good to go now (after passing the CI).

@pfitzseb pfitzseb merged commit 339eb78 into JuliaLogging:master Oct 21, 2019
@pfitzseb
Copy link
Member

Awesome, thanks!

@tkf tkf deleted the readme branch October 21, 2019 19:00
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.

4 participants