Skip to content

Readme badges and pyproject cleanup #3378

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

martinhoyer
Copy link
Collaborator

I'm not a friend with rst and have more experience with markdown. Will this render ok on PyPI page and other places? We also need to deal with man generation I presume?
PyPA should be ok based on:

$ twine check dist/tmt-1.40.0.dev2+ga31e8651-py3-none-any.whl --strict
Checking dist/tmt-1.40.0.dev2+ga31e8651-py3-none-any.whl: PASSED

github would look like this:
image

In pyproject, I've replaced Python 3.11 with 3.13 and removed the unmaintainable comments of what versions are available where.

Pull Request Checklist

  • implement the feature

@martinhoyer martinhoyer self-assigned this Nov 25, 2024
@martinhoyer martinhoyer added the code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix. label Apr 10, 2025
@psss psss added this to the 1.47 milestone Apr 10, 2025
@happz happz added this to planning Apr 10, 2025
@happz happz moved this to review in planning Apr 10, 2025
@martinhoyer
Copy link
Collaborator Author

image

Needs consensus on what badges are actually useful (which once to pick) and which of those make sense for them to be dynamic (versions, build successful).

I would say that Copr can be renamed to "Development/nighly builds" and just point to teemtee/latest, for example.

@happz
Copy link
Collaborator

happz commented Apr 10, 2025

image

Needs consensus on what badges are actually useful (which once to pick) and which of those make sense for them to be dynamic (versions, build successful).

I would say that Copr can be renamed to "Development/nighly builds" and just point to teemtee/latest, for example.

I'd probably drop "ruff" and "pre-commit", the rest will lead a user to some artifacts or docs, while these two are more like a statement, "yup, we test it", and we don't expect these to ever be red for main branch, correct?

@psss
Copy link
Collaborator

psss commented Apr 16, 2025

I'd probably drop "ruff" and "pre-commit", the rest will lead a user to some artifacts or docs, while these two are more like a statement, "yup, we test it", and we don't expect these to ever be red for main branch, correct?

Agreed, I feel it the same way.

Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Let's use the fresh copr link.

@psss psss moved this from review to implement in planning Apr 16, 2025
@thrix thrix requested a review from psss April 17, 2025 10:53
pyproject.toml Outdated
Comment on lines 34 to 43
"docutils>=0.16", # 0.16 is the current one available for RHEL9
"docutils>=0.16",
"fmf>=1.7.0",
"jinja2>=2.11.3", # 3.1.2 / 3.1.2
"packaging>=20", # 20 seems to be available with RHEL8
"pint>=0.16.1", # 0.16.1
"jinja2>=2.11.3",
"packaging>=20",
"pint>=0.16.1",
"pydantic>=1.10.14",
"pygments>=2.7.4", # 2.7.4 is the current one available for RHEL9
"requests>=2.25.1", # 2.28.2 / 2.31.0
"ruamel.yaml>=0.16.6", # 0.17.32 / 0.17.32
"urllib3>=1.26.5, <3.0", # 1.26.16 / 2.0.4
Copy link
Collaborator

Choose a reason for hiding this comment

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

@martinhoyer why to drop these comments? seems till useful ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thrix Well, it seems unmaintainable, definitely was a mistake to add Fedora versions there. Mea culpa.

I presume we can add "explanation" for the lock, like "version xyz available on epel9 [04/2015]" or something", but not sure if there is any value as that is the case for all of them: "Min versions are dictated by minimal versions of RHEL/EPEL9 because we don't have something like #3605, this is also why we have Python 3.9, even though it will be EOL in 6 months"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just realized they are also copy-pasted to .pre-commit-config twice.

@@ -182,7 +183,7 @@ template = "dev"
description = "Run scripts with multiple Python versions"

[[tool.hatch.envs.test.matrix]]
python = ["3.9", "3.11", "3.12"]
python = ["3.9", "3.12", "3.13"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

@martinhoyer I believe this is a significant change, it would desire it's own MR, ideally with the pyproject.yaml changes. Seems to be now hidden, I need to read the comments to find out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@thrix hmm, afaik this is not being used in any pipeline and only affects hatch run test:foo, isn't it?

@psss psss modified the milestones: 1.47, 1.48 Apr 23, 2025
@martinhoyer
Copy link
Collaborator Author

martinhoyer commented Apr 23, 2025

I'd probably drop "ruff" and "pre-commit", the rest will lead a user to some artifacts or docs, while these two are more like a statement, "yup, we test it", and we don't expect these to ever be red for main branch, correct?

Agreed, I feel it the same way.

@psss @happz I don't see what's wrong with that. It just tells people that this project uses pre-commit and ruff, which can increase the respect for the project, making them more likely to get involved. I mean, it's literally called a "badge".

Alas, removed as requested.

EDIT: oh and it can remind people to run pre-commit install. I tend to forget in new repos ;)

@martinhoyer martinhoyer moved this from implement to review in planning Apr 23, 2025
@martinhoyer martinhoyer added the ci | full test Pull request is ready for the full test execution label Apr 23, 2025
@martinhoyer
Copy link
Collaborator Author

The dependency version bumps shouldn't make any difference in theory, but just to be safe, added full_test label

@martinhoyer martinhoyer requested review from thrix and mcasquer April 29, 2025 14:39
@mcasquer
Copy link
Collaborator

@martinhoyer LGTM, just a thought: do you think we can point the python-versions badge to a different place than pypi? IMHO some place in the docs would be better although I haven't found anything apart from a brief mention in the Unit Tests section

@happz
Copy link
Collaborator

happz commented Apr 30, 2025

Seems to be rendering well in both HTML docs and man page: neither of these two shows any sign of badges :/ Which I guess is fine, yet a bit surprising.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution code | trivial A simple patch - a couple of lines, an easy-to-understand change, a typo fix.
Projects
Status: review
Development

Successfully merging this pull request may close these issues.

5 participants