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

Sensible logging #53

Merged
merged 7 commits into from
Sep 11, 2019
Merged

Sensible logging #53

merged 7 commits into from
Sep 11, 2019

Conversation

jgomezdans
Copy link
Collaborator

Addresses #5
Addresses #38
Addresses #33

OK, so I added some logging infrastructure to the entire codebase. As far as I can see, it works as advertised, other than the console logger outputs stuff twice. I haven't been able to trap what causes that (e.g. adding an extra handler but I can't see anywhere this is done).

There's some extra flexibility one may want to have (e.g. set the level via a verbose flag), but that's a cosmetic change.

Jose Gomez-Dans added 6 commits September 6, 2019 13:06
Just some changes I copied over from another project I'm involved with.
Does this look sensible? Haven't tested in this code base yet, but if
OK, then we can quickly change prints and whatever to use this.

Addresses #5
Addresses #38
Addresses #33
I have now added logging both to a file and console. The code is
consistent for all submodules. It's spitting things twice to the
console, so there's some issue in adding handlers, but other than
that, it appears to be working OK.

There are lots of changes to all files, but basically most of them
are just simplifying things.
@jgomezdans jgomezdans added the enhancement New feature or request label Sep 10, 2019
@jgomezdans jgomezdans requested a review from ilectra September 10, 2019 18:03
Copy link
Member

@dpshelio dpshelio left a comment

Choose a reason for hiding this comment

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

I would say to create an issue for double logging as I don't see where or how it happens. So we can fix later. Rest seems ok.

Co-Authored-By: David Pérez-Suárez <[email protected]>
@dpshelio dpshelio merged commit 520ab88 into master Sep 11, 2019
@dpshelio dpshelio deleted the sensible_logging branch September 11, 2019 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants