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

Cleaning up #489's implementation #503

Merged
merged 2 commits into from
Sep 28, 2024
Merged

Cleaning up #489's implementation #503

merged 2 commits into from
Sep 28, 2024

Conversation

jamesbraza
Copy link
Collaborator

@jamesbraza jamesbraza commented Sep 28, 2024

I felt my implementation in #489 was bad:

  • _PAPERQA_ROOT_LOGGER was a vague name -- unclear if "root" or "paperqa"

  • It's not clear the RichHandler is getting installed, so I made a flag for it

  • This was unintuitive, as it's true once at the start of CLI

    if not is_running_under_cli():
        _PAPERQA_ROOT_LOGGER.addHandler(rich_handler)

@jamesbraza jamesbraza added the enhancement New feature or request label Sep 28, 2024
@jamesbraza jamesbraza self-assigned this Sep 28, 2024
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Sep 28, 2024
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 28, 2024
@jamesbraza jamesbraza merged commit e6ab66f into main Sep 28, 2024
5 checks passed
@jamesbraza jamesbraza deleted the cleanups branch September 28, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants