Skip to content

Chore/recover linters - #371

Merged
DavidMinarsch merged 6 commits into
mainfrom
chore/recover-linters
Oct 14, 2022
Merged

DavidMinarsch merged 6 commits into
mainfrom
chore/recover-linters

Conversation

@dvilelaf

@dvilelaf dvilelaf commented Oct 14, 2022 •

Copy link
Copy Markdown

Proposed changes

  • Fixes some linters
  • Recovers the api docs check
  • Runs api doc generator (therefore changes +120 files)

Fixes

n/a

Types of changes

What types of changes does your code introduce to agents-aea?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING doc
  • I am making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Lint and unit tests pass locally with my changes and CI passes too
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that code coverage does not decrease.
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Comment on lines +196 to +197
- name: Check API Docs updated
run: tox -e check-api-docs

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Recovering the check-api-docs

Comment thread .pylintrc
Comment on lines +24 to +36
# C0209: consider-using-fstring, # kept as no harm
# W1514: unspecified-encoding, # kept as no harm
# R1734: use-list-literal, # kept as no harm
# R1735: use-dict-literal, # kept as no harm
# R1732: consider-using-with
# W0237: arguments-renamed
# R1733: unnecessary-dict-index-lookup
# R1710: inconsistent-return-statements
# R1714: consider-using-in
# R1729: use-a-generator
# R0402: consider-using-from-import
# W0640: cell-var-from-loop
# C0206: consider-using-dict-items

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

All these were introduced between pylint 2.6.0 to 2.11.1. Now we have introduced Tomte and making that upgrade, we do not want to change our linter settings so these must be skipped.

Comment thread aea/helpers/async_utils.py Outdated
source = "file.wrong"
>
with self.assertRaises(Exception):
with self.assertRaises(ValueError):

@angrybayblade angrybayblade Oct 14, 2022 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why was this changed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Pylint informed that this check was too generic. I could skip the failure, but it seemed very easy to solve. Would you prefer to revert this change and skip the lint error?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the test pass no need to revert the change

name=self.__class__.__name__ # type: ignore # loop was set in set_loop
)
self._thread.setDaemon(True)
self._thread.daemon = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Was this an api change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

A deprecation: #371 (comment)

@DavidMinarsch
DavidMinarsch merged commit 8334132 into main Oct 14, 2022
@dvilelaf
dvilelaf deleted the chore/recover-linters branch October 14, 2022 12:00
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.

4 participants