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.

Sign up for GitHub

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

Jump to bottom

Updated protobuf and jsonschema. #2813

Closed
wants to merge 9 commits into from
Closed

Updated protobuf and jsonschema. #2813

wants to merge 9 commits into from

Conversation

mariacarmina
Copy link

@mariacarmina mariacarmina commented Nov 10, 2022 •
edited
Loading

Proposed changes

Updated protobuf and jsonschema versions for further development.

Fixes

Fixes #2814

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

@mariacarmina
Copy link
Author

@solarw I saw that you opened a PR which is related with what I described in #2814. I have done some new changes on this branch and I was wondering if you can approve the CI workflows to run please. Thanks!

solarw
solarw reviewed Nov 18, 2022
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
solarw
solarw reviewed Nov 18, 2022
View reviewed changes
Copy link
Contributor

@solarw solarw left a comment

Choose a reason for hiding this comment

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

LGTM, couple of small comments and waiting till 0.6.1

5A11
5A11 requested changes Nov 21, 2022
View reviewed changes
Copy link
Member

@5A11 5A11 left a comment

Choose a reason for hiding this comment

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

The PR should be based off of and against developand not main.

@5A11
Copy link
Member

5A11 commented Nov 21, 2022

@mariacarmina Thanks for your PR. Please address the above comments, otherwise should be ready to go.

@mariacarmina mariacarmina requested review from solarw and 5A11 and removed request for solarw November 22, 2022 12:28
@mariacarmina mariacarmina changed the base branch from main to develop November 22, 2022 12:28
@mariacarmina
Copy link
Author

The PR should be based off of and against developand not main.

I have changed the base to develop.

@mariacarmina mariacarmina requested review from solarw and 5A11 and removed request for 5A11 and solarw November 22, 2022 12:31
5A11
5A11 reviewed Nov 22, 2022
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@mariacarmina
Copy link
Author

I have updated the poetry.lock file. The CI was complaining about python-mbedtls (==2.2.0) package which was present before.

@mariacarmina
Copy link
Author

mariacarmina commented Nov 22, 2022 •
edited
Loading

The remaining failing tests are: python code quality checks score which is 9.99/10 and golang checks on MacOS system. I have searched the error for golang, but the solution for it, it's related to re-compile the go files or something similar. In order to do that, I exported the PATH env var for go installation as I saw here

@5A11
Copy link
Member

5A11 commented Nov 23, 2022

@mariacarmina thanks for your contribution. CI is failing on some pylint issue.
In the interest of time, we have created a PR #2815 that passes CI (mostly except flaky tests) and achieves the same. Do you want to have a look and if you are happy approve?

@mariacarmina
Copy link
Author

@mariacarmina thanks for your contribution. CI is failing on some pylint issue. In the interest of time, we have created a PR #2815 that passes CI (mostly except flaky tests) and achieves the same. Do you want to have a look and if you are happy approve?

yes, I can take a look over that PR, sure. Thanks for letting me know.

@5A11
Copy link
Member

5A11 commented Nov 23, 2022

Thanks again for your contribution @mariacarmina and reviewing #2815. That PR is now merged and so this can be closed.

@5A11 5A11 closed this Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@solarw solarw solarw left review comments

@5A11 5A11 Awaiting requested review from 5A11 5A11 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: Update protobuf & jsonschema dependencies
3 participants
@mariacarmina @5A11 @solarw