Black formatting preparation #3314
Conversation
Signed-off-by: Coady LaCroix <[email protected]>
Signed-off-by: Coady LaCroix <[email protected]>
clacroix12
requested review from
andrewschoen,
zmc,
vasukulkarni,
mbukatov and
a team
ocs_ci/utility/gcp.py
Outdated
| # cluster access as well. | ||
| # | ||
| # For more details, see `GCP documentation on ServiceAccountKey resource | ||
| # <https://cloud.google.com/iam/docs/reference/rest/v1/projects.serviceAccounts.keys>`_ |
There was a problem hiding this comment.
Would this change preserve this docstring in our sphinx build?
Right now, it's part of our documentation as expected: https://ocs-ci.readthedocs.io/en/latest/apidoc/ocs_ci.utility.html#ocs_ci.utility.gcp.SERVICE_ACCOUNT_KEY_FILEPATH
There was a problem hiding this comment.
Hmm, this I'm not sure of. Right now it's causing the following error in our check-docstring-first hook:
Check docstring is first.................................................Failed
- hook id: check-docstring-first
- exit code: 1
ocs_ci/utility/gcp.py:29 Multiple module docstrings (first docstring on line 2).
There was a problem hiding this comment.
I think we can check the docs from this build and resolve it if it looks good
There was a problem hiding this comment.
There is actually an open issue for this pre-commit/pre-commit-hooks#159 with a potential fix that has yet to be merged. It may be worth reconsidering the use of check-docstring-first until that is fixed if we want to use attribute docstrings.
There was a problem hiding this comment.
Actually, looks like there may be a workaround for this: pre-commit/pre-commit-hooks#159 (comment). I'll try to test this out.
There was a problem hiding this comment.
This is a bug in check-docstring-first, likely pre-commit/pre-commit-hooks#159 - as you can see in the html sphinx build, both module docstring and variable docstring are present as expected.
I suggest to disable this particular hook.
There was a problem hiding this comment.
Yeah, I have tried to use the #: syntax workaround but it doesn't seem to work for me. I'm not seeing any attribute doc unless I go with the triple quote docstring method.
I suggest to disable this particular hook.
I agree, I am going to revert the change to this docstring and disable the check entirely.
clacroix12
force-pushed
the
black-prep
branch
from
c37179c to
6dca5ab
Compare
Signed-off-by: Coady LaCroix <[email protected]>
…ng sphinx attribute docstrings Signed-off-by: Coady LaCroix <[email protected]>
clacroix12
force-pushed
the
black-prep
branch
from
6dca5ab to
dcaac05
Compare
|
@vasukulkarni @mbukatov I have reverted the change to that particular docstring and removed |
* Flake8 ignore E203 * Fix flake8 issues * Fix check-docstring-first failures * Remove check-docstring-first pre-commit hook as it doesnt support using sphinx attribute docstrings Signed-off-by: Coady LaCroix <[email protected]>
This PR fixes various issues that were causing pre-commit to fail after formatting the repo with black. These failures were coming up now because pre-commit runs against files in your changeset and not against the entire repository. These problems existed before black formatting was being done.