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

Chore/recover linters #371

Merged
merged 6 commits into from
Oct 14, 2022
Merged

Chore/recover linters #371

merged 6 commits into from
Oct 14, 2022

Conversation

dvilelaf
Copy link
Collaborator

@dvilelaf dvilelaf commented Oct 14, 2022

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
Collaborator 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 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
Collaborator 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.

aea/helpers/async_utils.py Outdated Show resolved Hide resolved
@@ -277,7 +277,7 @@ def test_extract_wrong_file_type(self):
"""Test for extract method wrong file type."""
source = "file.wrong"
target = "target-folder"
with self.assertRaises(Exception):
with self.assertRaises(ValueError):
Copy link

@angrybayblade angrybayblade Oct 14, 2022

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
Collaborator 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?

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

@@ -412,7 +412,7 @@ def start(self) -> bool:
self._thread = Thread(
target=self._thread_target, name=self.__class__.__name__ # type: ignore # loop was set in set_loop
)
self._thread.setDaemon(True)
self._thread.daemon = True

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
Collaborator 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