Development environment improvements

[adam-zlatniczki]

Is your feature request related to a problem? Please describe. Contributing to the repo is much harder than an outsider would like it to be. Python dependency hell doesn't help the case either, I've spent hours trying to figure out what's the issue with my setup - I'm still facing a bunch of problems coming most likely from soft version conflicts (for example, I can't run my tests in an IDE, which makes the debugging process time consuming). Another issue is the CI. The fact that there's a CI loop is a great start, but it would be nice to be able to run it locally without having to copy-paste commands from the GitHub workflow description. Also, having the same environment for development which is used later for continuous integration would also decrease the number of unnecessary iterations a contributor has to make before actually passing the CI. As this is an open-source project, I think it would make sense for it to provide the necessary tools to contributors.

Describe the solution you'd like A few things that would make contribution much easier:

• A devcontainer (https://code.visualstudio.com/docs/devcontainers/containers) that works out-of-the-box, having all the relevant tools included and preconfigured, so that contributors only have to have Docker and VSCode installed.
• Having exact python package versions defined for the development environment. I've seen from the pyproject.toml that you're already familiar with the poetry dependency manager, so I'd migrate the whole project on top of that. Version controlling the poetry lockfile would solve the dependency hell right away.
• A CI tool that can be run locally as well. I'd propose to use Bob, which can be run locally. The GitHub workflow would only have to call the Bob rules, making the whole project somewhat CI-agnostic.
• I'd also propose to change to the Ruff linter (https://github.com/astral-sh/ruff). It's written in Rust, and it supports many Python linter rules, including pylint (https://docs.astral.sh/ruff/rules/#pylint-pl), but it's so much faster that you can use it for seemless real-time linting in VSCode for example.

Describe alternatives you've considered

• If the Bob tool seems to be a bit risky choice due to its GitHub commit activity, make can be used in the same way (as you already do for tests). The only reason I proposed Bob is because it's YAML file is easier to read.
• The Ruff package also has some risk, pylint can still be used at the cost of extra runtime.

If you find the ideas relevant, we can have a discussion about them. Also, in case you agree with them and need help with implementing the changes, I can lend a hand.

[liran-funaro]

@adam-zlatniczki I greatly appreciate your effort and suggestions. My apologies for the hours you spent on this. I want the development process to be as simple as possible to encourage contributors. Would you mind elaborating on the issues you faced and how you resolved them?

Concerning your suggestions,

Having exact Python package versions defined for the development environment.

I think this is a good idea. It is, however, not my intention to impose unnecessary restrictions on the packages. Input on the exact packages that cause you problems will be greatly appreciated.

A CI tool that can be run locally as well

I have a tiny script that I developed for another project that runs the CI locally. It might be useful to add it to this project.

I'm not familiar with Bob, and I never saw a large-scale Python project that used it. I prefer using widely acceptable solutions so that experienced developers will be familiar with the environment.

change to the Ruff linter

I didn't know about this linter, and it seems to be common. I'd look into it. If you don't mind, can you please create an issue for this? Describing the issue with the current linter for this project, and why Ruff will be more suitable.

A devcontainer

Since this is a pure Python package, I think this is overkill. An experienced Python developer should be familiar with Python's virtual environment, which will solve any package versioning issues.

[liran-funaro]

After further reviewing the Ruff linter, I agree it is a better alternative to the current approach. If you are already familiar with Ruff and have time, it would be great if you could publish a PR that replaces the current linters (flake, pylint, and black) with Ruff.

[adam-zlatniczki]

I think this is a good idea. It is, however, not my intention to impose unnecessary restrictions on the packages. Input on the exact packages that cause you problems will be greatly appreciated.

I agree, but not entirely. Version constraints should in my opinion be kept in mind, at least up to a point where interface collisions are prevented. For example, here's what I encountered:

Exception occurred: File "sphinx-markdown-builder/sphinx_markdown_builder/builder.py", line 48, in init super().init(app, env) TypeError: Builder.init() takes 2 positional arguments but 3 were given

This happens if you have sphinx<5.1.0, which is unfortunately allowed by the currently expected sphinx>=2.2.0

However, sphinx>=5.1.0 requires python>=3.8, which conflicts with the current python>=3.7. This seems to be only a soft conflict, as indicated by the passing tests. However, if you don't use pip (which doesn't really care about dependency resolution and can result in an inconsistent, even faulty virtualenv) to install sphinx-markdown-builder, but use poetry for example, it will raise an issue unless python>=3.8 is enforced.

I also had to play around a little with the dev dependency group for similar reasons. If you have a fresh virtualenv with nothing else already installed, then the current approach works fine. But if you have an existing env where some dependencies already exist with improper versions, there can be several conflicts that have to be resolved. Without specifying the lowest required dependency versions, common tools like poetry or conda won't be able to do their resolve magic well.

I wouldn't propose to specify exact versions - that would pretty much kill the possibility of widespread use of this package. Lower bounds on the package versions are perfectly fine as they are (except for sphinx I guess, as pointed out above). But I'd introduce such lower bounds for the extra dependencies for development too (which I did locally, if you're interested I can share a set of constraints that works for the time being).

I have a tiny script that I developed for another project that runs the CI locally. It might be useful to add it to this project. I'm not familiar with Bob, and I never saw a large-scale Python project that used it. I prefer using widely acceptable solutions so that experienced developers will be familiar with the environment.

Perfectly fair point, Bob could definitely be a risk. Moving things to Make would be a fine solution in my opinion - did you use that for running the CI locally in that separate project? Or did you come up with a solution that parses the GitHub workflow and runs the commands present there? What's your take on moving all the steps from the CI yaml to the Makefile and simply invoking that within the workflow?

Since this is a pure Python package, I think this is overkill. An experienced Python developer should be familiar with Python's virtual environment, which will solve any package versioning issues.

This is something I disagree with, but hey, it's not my project, I can accept your decision :)

If you are already familiar with Ruff and have time, it would be great if you could publish a PR that replaces the current linters (flake, pylint, and black) with Ruff.

Sure thing. I'm a bit busy these days, but I'll try to set this up soon.

[liran-funaro]

This happens if you have sphinx<5.1.0, which is unfortunately allowed by the currently expected sphinx>=2.2.0

I modied the requirement to sphinx>=5.1.0.

However, sphinx>=5.1.0 requires python>=3.8, which conflicts with the current python>=3.7

They require Python 3.6 (https://github.com/sphinx-doc/sphinx/blob/v5.1.0/setup.py)

if you're interested I can share a set of constraints that works for the time being

That will be great!

did you use that for running the CI locally in that separate project? Or did you come up with a solution that parses the GitHub workflow and runs the commands present there?

It parses it.

What's your take on moving all the steps from the CI yaml to the Makefile and simply invoking that within the workflow?

I added make lint and make test rules.

Sure thing. I'm a bit busy these days, but I'll try to set this up soon.

Thanks! I'm looking forward to it.