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

added docstring to logging.py and added pydocstyle to github workflow #158

Closed
wants to merge 2 commits into from

Conversation

ritikaaaa177
Copy link
Contributor

@ritikaaaa177 ritikaaaa177 commented Aug 3, 2024

Hi, @refeed I have added a docstring to logging.py and also added pydocstyle to github workflow. Do checkout and let me know am i going right?

Copy link
Member

@refeed refeed 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 initiative! This is a good start, just one comment for now

.pydocstyle Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Aug 12, 2024

@ritikaaaa177
Copy link
Contributor Author

Hi @refeed made a commit, please check.


Here, if `verbose` is set to ``True``,then all the log messages higher than DEBUG level will be displayed in `verbose` format.
But, if `verbose` is set to ``False``, all the log messages that are higher than INFO level i.e. ('INFO', 'WARNING', 'CRITICAL') will be dsiplayed in `normal` format.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the detailed docstring! but I think it can be made shorter

I'd write it as the following:

    Set up logging configuration.

    :param verbose:  If set to `True` will log all messages higher than DEBUG level, otherwise it will log the messages that are higher than INFO level ('INFO', 'WARNING', 'CRITICAL')

to control the verbosity and formatting of log messages. The ``setup_logging`` function
allows switching between verbose and normal logging modes.

"""
Copy link
Member

Choose a reason for hiding this comment

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

We can remove the It provides a default logging configuration with different handlers and formatters to control the verbosity and formatting of log messages. The ``setup_logging`` function allows switching between verbose and normal logging modes. part

@@ -38,6 +48,14 @@


def setup_logging(verbose: bool = False):

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this newline

Copy link
Member

@refeed refeed left a comment

Choose a reason for hiding this comment

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

Also if you could do a rebase to the latest main, it'd be great. Otherwise, it's fine, we'll just rebase it when merging

@refeed
Copy link
Member

refeed commented Oct 16, 2024

Merged this via #188 . Thanks for the contribution!

@refeed refeed closed this Oct 16, 2024
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.

3 participants