Skip to content

Conversation

@ydirson
Copy link
Contributor

@ydirson ydirson commented Jul 11, 2022

pylint will be of help for python3 support.

ydirson added 8 commits July 11, 2022 17:39
Top-level xcp/ is not a symlink any more, it's a dir.

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
_CMPProxy is an abstract class, and pylint needs a way to see that, or it
issues an error:

 ************* Module xcp.cpiofile
 E:519,12: Instance of '_CMPProxy' has no 'init' member (no-member)

Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Copy link
Contributor

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

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

Thanks for enabling pylint. Could we get it to run using github actions?
See here what SM does: https://github.com/xapi-project/sm/blob/master/.github/workflows/main.yml

Might be useful to have a test matrix there: test using both python2.7 and 3.x to ensure our changes are compatible with both.

self.init()
self.read(pos - self.pos)

def init(self):
Copy link
Contributor

@edwintorok edwintorok Jul 12, 2022

Choose a reason for hiding this comment

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

I think this can be done via some of the @abc annotations in python, would https://docs.python.org/3/library/abc.html#abc.abstractmethod (and inheriting from the proper ABC class) fix the pylint warning too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, using abc works just as well. That one is annoying in projects making heavy use of metaclasses (which is why I default to the exception idiom), but it will do the job here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note, inheriting from abc.ABC comes with 3.4 only, so we'd have to use something like @six.add_metaclass(abc.ABCMeta) for a 2/3-portable idiom, which I feel brings too much complexity, for no clear advantage over NotImplementedError(). What do you think ?

Choose a reason for hiding this comment

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

Note, inheriting from abc.ABC comes with 3.4 only, so we'd have to use something like @six.add_metaclass(abc.ABCMeta) for a 2/3-portable idiom, which I feel brings too much complexity, for no clear advantage over NotImplementedError(). What do you think ?

I think I'd concur with that. My feeling is we want the Py2/Py3 compatibility phase to be relatively short until the migration is complete and then drop the py2 support completely at which point there can be a follow on set of changes to perform code cleanups.

@ydirson
Copy link
Contributor Author

ydirson commented Jul 13, 2022

Thanks for enabling pylint. Could we get it to run using github actions? See here what SM does: https://github.com/xapi-project/sm/blob/master/.github/workflows/main.yml

sure!

Might be useful to have a test matrix there: test using both python2.7 and 3.x to ensure our changes are compatible with both.

That does make sense for unit tests, but for pylint the usefulness will I think mostly be in making use of newer py3-only versions of pylint - all of which requires to get the migration far enough first.

@ydirson ydirson force-pushed the pylint branch 4 times, most recently from b4fda60 to 54494b4 Compare July 15, 2022 09:34
@edwintorok
Copy link
Contributor

I think to enable the CI we'd have to merge this to master, I see it passed on your fork

ydirson added 2 commits July 15, 2022 12:37
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
Signed-off-by: Yann Dirson <yann.dirson@vates.fr>
self.init()
self.read(pos - self.pos)

def init(self):

Choose a reason for hiding this comment

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

Note, inheriting from abc.ABC comes with 3.4 only, so we'd have to use something like @six.add_metaclass(abc.ABCMeta) for a 2/3-portable idiom, which I feel brings too much complexity, for no clear advantage over NotImplementedError(). What do you think ?

I think I'd concur with that. My feeling is we want the Py2/Py3 compatibility phase to be relatively short until the migration is complete and then drop the py2 support completely at which point there can be a follow on set of changes to perform code cleanups.

@edwintorok edwintorok merged commit 316c738 into xenserver:master Jul 15, 2022
@ydirson ydirson deleted the pylint branch July 15, 2022 15:06
liulinC added a commit that referenced this pull request Apr 16, 2024
CP-48784: Sync with original tar file
bernhardkaindl pushed a commit to rosslagerwall/python-libs that referenced this pull request May 8, 2024
bernhardkaindl added a commit that referenced this pull request Aug 15, 2025
* tox.ini: Complete the transition from pyre to pyright

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* pre-commit: Remove the remaining call to pyre in pre-commit

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* docs: Complete the transition from pyre to pyright

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `tox.ini/pyright`: Fail `tox` if `pyright` fails (enforce check)

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* pyre: Finally, remove obsolete pyre error suppressions

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* Fix remaining pyright warnings found by the unit tests

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* GitHub CI/`tox`: Update CI to use Python `3.11`, `3.12` and `3.13`

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `CONTRIBUTING.md`: Add `venv` setup and using `pipx`

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `tests/test_logger.py`: Use `pyfakefs`: don't create a file in the host

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `.github/workflows/main.yml`: Cleanup obsolete code for Python 2.7/3.8

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `CONTRIBUTING.md`: Fix Markdown format for linting

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `README.md`: Fix Markdown format for linting and update it

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `README-Unicode.md`: Fix Markdown format for linting and update it

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* `.pre-commit-config.yaml`: Replace mdformat check with markdownlint-cli

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* pytest.ini: Support newer pytest versions (>=7) to fix errors devs face

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

* tox.ini: Fix choking on venvs with symlink to dirs: skip untracked files

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>

---------

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@cloud.com>
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.

3 participants