Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Jun 24, 2015

Also

  • Updated docstrings which don't pass / are invalid.
  • Integrated plugin with tox lint rule and run_pylint.
  • Fixed a elif _GAECreds is not None and isinstance(credentials, _GAECreds): in gcloud.credentials._get_service_account_name.

For now, pulling directly from the mercurial source repository to get the check_docs pylint extension. The maintainer mentioned this will be released July 15-17, 2015.

See:
http://www.bitbucket.org/logilab/pylint/pull-request/143/

Also
- Updated docstrings which don't pass / are invalid.
- Integrated plugin with `tox lint` rule and `run_pylint`.

For now, pulling directly from the mercurial source
repository to get the `check_docs` pylint extension. The
maintainter mentioned this will be released July 15-17, 2015.

See:
http://www.bitbucket.org/logilab/pylint/pull-request/143/
@review-ninja
Copy link

ReviewNinja

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 24, 2015
@dhermes
Copy link
Contributor Author

dhermes commented Jun 24, 2015

@tseaver Sorry about reviewninja. I was toying around with it and didn't realize it had privileges to comment on GitHub. I have now disabled that.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@tseaver
Copy link
Contributor

tseaver commented Jun 24, 2015

This is kind of an ugly, wide-ranging change to be merging in the middle of other development (it is going to cause conflicts for pretty much any WIP / open PR.

@dhermes
Copy link
Contributor Author

dhermes commented Jun 25, 2015

@tseaver I'm with you on most of that (not sure I buy that it's ugly, but some of the diffs are hard to grok).

But do we ever foresee a time where we won't be in the middle of development? This likely won't cause merge issues because it touches docstrings whereas most code changes will touch code.

Did you have your dataset -> client rename in mind or something else?

As for open PRs, it is definitely going to be an issue, but again, one we can't really avoid. As you can see from these changes, we had a lot of docstrings that weren't actually documenting the methods / classes / functions correctly.

@dhermes
Copy link
Contributor Author

dhermes commented Jun 25, 2015

@tseaver How do you see a merge going? I assume you finished your review?

@tseaver
Copy link
Contributor

tseaver commented Jun 25, 2015

If you can stand the merge problems this issue creates, go ahead and merge (I would be tempted to defer this until a "janitorial" phase, myself).

@dhermes
Copy link
Contributor Author

dhermes commented Jun 25, 2015

dhermes added a commit that referenced this pull request Jun 25, 2015
Adding Pylint plugin to check Sphinx docstrings.
@dhermes dhermes merged commit 7dece69 into googleapis:master Jun 25, 2015
@dhermes dhermes deleted the add-pylint-docstring-checker branch June 25, 2015 18:05
@dhermes dhermes mentioned this pull request Jul 10, 2015
parthea pushed a commit that referenced this pull request Sep 18, 2025
parthea pushed a commit that referenced this pull request Nov 22, 2025
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 24, 2025
* fix: remove custom retry loop

* removed async implementation

* removed unneeded imports

---------

Co-authored-by: Kevin Zheng <147537668+gkevinzheng@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 24, 2025
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 24, 2025
parthea pushed a commit that referenced this pull request Nov 25, 2025
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
parthea pushed a commit that referenced this pull request Nov 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants