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

Add mypy checks #5

Closed
wants to merge 6 commits into from
Closed

Add mypy checks #5

wants to merge 6 commits into from

Conversation

RLKRo
Copy link
Member

@RLKRo RLKRo commented Oct 20, 2022

As mentioned here viewing codestyle check results is not convenient.

This pull request solves that problem in two ways:

  1. codestyle1.yml -- produces the following output.
  2. codestyle2.yml -- produces the following output.

In my opinion, the second solution is better.

@RLKRo RLKRo requested review from kudep and pseusys October 20, 2022 09:12
@pseusys
Copy link
Collaborator

pseusys commented Oct 20, 2022

Second option is more obvious for sure. However:

  1. Checkout and triple dependencies installation are required
  2. What's the difference at all if in the end all checks should pass?..

I would suggest either leave the first one or use second with some dependency caching.

kudep added a commit that referenced this pull request Oct 21, 2022
* add ydb_connector

* lint

* ydb docker

* add setup extra

* feature/ydb_connector: fix merge

* feature/ydb_connector: massive upd, wip

* feature/ydb_connector: fix sql, mongo, redis

* fix: adapt ydb contexts serialization for uuid (#5)

* fix: change context id to str type

* fix: change id type to utf8

* fix: change serialization method for uuid

Co-authored-by: Denis Kuznetsov <kuznetsov.den.p@gmail.com>

* feature/ydb_connector: fix ydb

* feature/ydb_connector: fix style

* feature/ydb_connector: rm useless files & upd examples

Co-authored-by: Denis Kuznetsov <kuznetsov.den.p@gmail.com>
Co-authored-by: Evgeniy Egorov <90514210+EvgeniyEgoorov@users.noreply.github.com>
@kudep
Copy link
Collaborator

kudep commented Oct 23, 2022

Second option is faster then our current unit-tests (because of that we do not have to wait three linting tests) and it's more atomic style. So I prefer the second one.

@RLKRo RLKRo marked this pull request as ready for review October 24, 2022 09:29
@kudep
Copy link
Collaborator

kudep commented Oct 24, 2022

We will wait until the end of addon's mega-merges.

@kudep kudep added this to the Release 0.2.0 milestone Nov 10, 2022
@RLKRo RLKRo added the enhancement New feature or request label Nov 10, 2022
@kudep kudep modified the milestones: Release 0.2.0, Release 0.3.0 Nov 22, 2022
@kudep
Copy link
Collaborator

kudep commented Jan 27, 2023

Do we need this PR, i think it can be closed

@RLKRo
Copy link
Member Author

RLKRo commented Jan 27, 2023

I think it should be used to add mypy check. The name of the branch is codestyle/add-mypy-check. So should probably just rename this PR.

@RLKRo RLKRo changed the title Decide how to do codestyle checks Add mypy checks Jan 27, 2023
@RLKRo RLKRo marked this pull request as draft April 19, 2023 14:15
@kudep kudep removed this from the Release 0.2.0 milestone Sep 26, 2023
@RLKRo RLKRo closed this Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants