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

Add docstring for each procedure and method for pelican/__init__.py #3359

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

egberts
Copy link
Contributor

@egberts egberts commented Jul 5, 2024

Pull Request Checklist

Add missing docstrings to pelican/__init__.py.

Zero code change.

Just a rough cut to see if the Pelican community are up for that. ( I know some folks like comment-less code so treading lightly here).

  • Ensured tests pass and (if applicable) updated functional test output
  • Conformed to code style guidelines by running appropriate linting tools
  • Added tests for changed code
  • Updated documentation for changed code

egberts added 2 commits July 5, 2024 09:27
Setting terminal screen to 80x24, test the docstring using:

    python -c "import pelican;help(pelican)"

Tiny adjustments to ensure that all comment lines are under 70-columns.
@boxydog
Copy link
Contributor

boxydog commented Jul 13, 2024

I am only one voice, and I don't have much influence over whether this gets merged, but ..

I'd be concerned that the long comments would get out of date. Anything that is not unit tested or with eyes on it eventually breaks. For example, the very long comments (e.g., at the top, in __init__, etc.) seem more likely to be out of date to me. So I'd want to hear more about what sort of comments might be likely to remain correct over the long term.

Also, I'd be more interested in type hints, for example. Instead of ":returns None", how about "-> None" on the function signature. Those are also not currently checked, but at least I'd see if they were wrong in my IDE.

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