Open-Wine-Components / umu-protonfixes

BSD 2-Clause "Simplified" License
58 stars 32 forks source link

TODO: Extend lint rules for Pylint #50

Open R1kaB3rN opened 6 months ago

R1kaB3rN commented 6 months ago

Related to https://github.com/Open-Wine-Components/umu-protonfixes/pull/48#issuecomment-2025754406

For quality assurance and to prevent bugs such as https://github.com/Open-Wine-Components/umu-protonfixes/commit/ac9ddf2660f4cd79ddc157424ee238926a4d6d30, more lint rules should be extended and applied to the Protonfix/gamefix modules.

Root-Core commented 6 months ago

If I don't misunderstand you, this is already the case with commit c956ac9689e59391c6df3380e7e18271809c6520, the , adds the files on the root level.

Well, we could just enforce it on all *.py files - without the "static" array.

R1kaB3rN commented 6 months ago

If I don't misunderstand you, this is already the case with commit c956ac9, the , adds the files on the root level.

Sorry, my description was sloppy/inaccurate there. Thanks for the correction.

Well, we could just enforce it on all *.py files - without the "static" array.

Yeah and probably define some files to exclude such as __init__.py for the gamefixes in pyproject.toml

Root-Core commented 6 months ago

No problem. I moved the linked comment here for better discussion. I also expanded it a bit.


We should enforce this via linter:

We want to:

R1kaB3rN commented 5 months ago

I think enforcing rules for Docstrings will require additional tools than pylint. Personally, I'd like to just have one tool for the sake of simplicity.

In addition, there should probably as much tests as possible. In particular for critical files such as fix.py and util.py

R1kaB3rN commented 1 month ago

I have a PR open that's addresses some of these issues.

As for these items:

I've already confirmed that the GDrive functionality does not work, but I believe it would be best to address them in a separate PRs where we can consider removing some functionality. In fact, there shouldn't be a need to support Google drive anyway as users can upload files to Github.

Root-Core commented 4 weeks ago

EDIT: I had a look at your PR and it includes basically my changes.. like enforced types and removal of pylint-exceptions.. I will wait for it to be merged, before I continue doing bigger changes.

R1kaB3rN commented 4 weeks ago

Just merged the PR.

Yeah, I support removing the GDrive functionality. Same with splitting the modules, which will be a large change. To that end, to reduce the likelihood of potential symbol errors, I think it would be worth adding a tool like pyright in the CI.

Root-Core commented 4 weeks ago

We should also implement a README with some hints and explanations. For example, how to run the checks locally, and what a good fix should look like / how the umu-id works for non-steam games (or links to the corresponding repo).

R1kaB3rN commented 4 weeks ago

We should also implement a README with some hints and explanations. For example, how to run the checks locally, and what a good fix should look like / how the umu-id works for non-steam games (or links to the corresponding repo).

Agreed, especially for running/testing checks locally which isn't as simple anymore. Since the merge of the build system, some binaries were removed, so users can't simply just replace a Proton build's entire protonfixes directory anymore for some cases. For example, protonfixes.util.get_resolution expects xrandr to be within the project directory for it to work.

Also, for users who want to add a non-Python library, they will be expected to compile the project within the Steam Runtime, copy the dist/protonfixes directory to a GE-Proton directory generated from make install, and move, if there are any, the *.so file files to the Steam Runtime's LD_LIBRARY_PATH. If that all works, then finally update the GE-Proton/UMU-Proton's Makefile to copy those files at build time. To improve the developer/tester UX, some of these steps should ideally be automated with a simple script.