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

Logging fixes #52

Merged
merged 14 commits into from
Nov 25, 2024
Merged

Logging fixes #52

merged 14 commits into from
Nov 25, 2024

Conversation

skyfenton
Copy link
Collaborator

Fleshed out some details with the cli/logging including creating an output directory if one doesn't exist and naming logs based on when the program is executed instead of just appending to an existing log.

@skyfenton skyfenton requested a review from audiodude November 24, 2024 07:59
@skyfenton skyfenton marked this pull request as draft November 24, 2024 08:01
@skyfenton skyfenton marked this pull request as ready for review November 24, 2024 08:14
Copy link
Collaborator

@audiodude audiodude left a comment

Choose a reason for hiding this comment

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

Looking good. Just found a few small things but nothing is terribly important.

mediabridge/main.py Outdated Show resolved Hide resolved
mediabridge/main.py Outdated Show resolved Hide resolved
mediabridge/main.py Outdated Show resolved Hide resolved
@skyfenton skyfenton marked this pull request as draft November 25, 2024 01:32
@skyfenton
Copy link
Collaborator Author

using logging_redirect_tqdm actually redirects to stdout when trying to write to a file, working on a fix

@skyfenton skyfenton marked this pull request as ready for review November 25, 2024 01:44
Copy link
Collaborator

@audiodude audiodude left a comment

Choose a reason for hiding this comment

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

Ending up more confused than I started, please help me out!

mediabridge/main.py Show resolved Hide resolved
mediabridge/main.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@audiodude audiodude left a comment

Choose a reason for hiding this comment

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

Thanks for the explanations!

@skyfenton
Copy link
Collaborator Author

Added those explanations to the code so it's clear for everyone, should be all set to merge afterwards unless there are any last changes.

@skyfenton skyfenton merged commit b4a97c3 into main Nov 25, 2024
4 checks passed
@skyfenton skyfenton deleted the logging-fixes branch November 25, 2024 23:11
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.

2 participants