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

Stop invoking setup.py directly #147

Merged
merged 7 commits into from
Oct 13, 2023

Conversation

anderslindho
Copy link

I saw that this project made use of setup.py as well as a number of requirements.txt files for setting up environments. The general recommendation is to no longer invoke setup.py directly (find a good recap here), so I've made some changes to the build configuration in case you're interested.

I have updated the instructions in the README but have not modified the CHANGELOG.

I also noted some more things as I was doing this:

  • You make use of flake8 for some linting and checking on formatting. I don't know if you have opinions, but I would recommend using black for formatting, and pre-commit for automatic formatting (and linting), and to integrate pre-commit with the CI/CD. I could set this up for you if you'd like.
  • The tests appear to have been deactivated because some of them require direct access to IO not existing on headless systems (like the CI/CD runners). I did some brief attempts to try and exclude certain tests (in order to reactivate tox in GH actions) but unsuccessfully so; if this would be of interest, however, I could try a bit further.
  • The badge for the documentation says that the docs are failing, both after and before my MR. Would you like me to look into this?

@einarf
Copy link
Member

einarf commented Nov 16, 2021

Thanks. The reason I've been delaying this change is a github bug that causes the repo to no longer be recognized as a python package losing all links to dependencies and projects depending on moderngl-window. If this is still an issue we still need a setup.py in the repo at the very least. I could try to merge this and see what happens,

@anderslindho
Copy link
Author

Thanks. The reason I've been delaying this change is a github bug that causes the repo to no longer be recognized as a python package losing all links to dependencies and projects depending on moderngl-window. If this is still an issue we still need a setup.py in the repo at the very least. I could try to merge this and see what happens,

That's curious. Do you want me to add back a minimal setup.py for compatability or leave it be?

@einarf
Copy link
Member

einarf commented Nov 16, 2021

Nah. Let's just see what happens. Should be easy to add a skeleton setup.py if needed 😄

Just need to find a good time. Very soon.

@einarf
Copy link
Member

einarf commented Nov 16, 2021

Note to self: Also check docs

@einarf
Copy link
Member

einarf commented Sep 17, 2022

Kind of forgot about this. Will merge it in next next release. I'll fix the conflcs, so just let this one linger.

@einarf
Copy link
Member

einarf commented Oct 13, 2023

I'll try to salvage this PR to respect the time you spent on this. Seems the simplest way it to merge in master, tweak a bit and squash it.

@einarf
Copy link
Member

einarf commented Oct 13, 2023

Merged two years later. Better now than never 😆

@einarf einarf merged commit b2b3150 into moderngl:master Oct 13, 2023
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.

2 participants