-
Notifications
You must be signed in to change notification settings - Fork 28
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
PEP621 and hatch
scripts
#202
base: main
Are you sure you want to change the base?
Conversation
We need to be able to build on epel-9 which fails now (very likely contains too old macros/utils). @psss WHat about epel-8 though, can we stop providing updates as we did with tmt? epel-8 has even older macros ;) |
Makefile contains useful stuff as 'make (s)rpm', 'make clean'. IMO we need leave it (it can call hatch commands though). |
We moved AWAY from it in the past. Easier to use libraries locally when python path works out of the box. |
Indeed, I was just about to look into that. I was debugging locally and I hit an issue with it just now about dynamic version. It's weird that the same setup is used in packit. That needs a bit of debugging.
Sure thing. I would still prefer the CI, packit and spec to use the direct commands though. Fine with that?
Roger. It is a dev preference in the end |
059f6d9
to
4d79f9c
Compare
I am not sure why Rhel9 build is failing. I can't seem to debug it to check if the tmp folder includes |
Seems like the error message in the JsonSchemaValidator is different on the other distros:
(note the lack of And similar with tmt errors reported. |
Yes, I'd say we should keep |
Ok, this should be ok for this PR. @psss can someone look into the testing-farm failures? They are in the integration test and it should just need a more relaxed check of the fail message. |
The testing farm failures were most probably caused by the fresh |
/packit test |
8ccf9b6
to
8ca3f85
Compare
@lukaszachy , @psss Ok, with this Epel 9 is fixed. Thanks to Nikola from packit for debugging the issue. I've also expanded the github tests, can someone trigger them? I've also enabled codecov, but on my project it did not work without a secret token. Maybe that needs to be setup here as well? |
/packit test |
@lukaszachy, @psss, @happz Review for this one please? |
Sorry, had a bunch of stuff on the plate so I did no get to this. Will have a look, hopefully this week. Sorry for the delay. |
Let's finish this as one of the first things for the |
Oh, I need to rebase (after #215), need anything else than that? |
Yes, please rebase. No other changes needed right now (at least from the quick look through the changes). |
@psss @lukaszachy @martinhoyer @happz What do you think of getting this in for The current status is:
Addressing #224 (40023ee) would be very useful before pushing and update, maybe I can cherry-pick that one out if needed. |
@LecrisUT Apologies, I'm afk for the rest of the week. Fwiw, I think it's in great state, but it might be a good idea to not rush it into 1.4 and instead maybe make 1.5 a dedicated release just for this? (thinking aloud) |
Thanks for putting this all together and for the nice summary! Had a quick look, sounds good, but it's not that little change and (also from the |
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments. One more thing I wonder about is how to generate manpage - if it make sense to have it same as https://github.com/teemtee/tmt/blob/54dfd29ede1e3a20bad98bb6bf8c79dc0da25e54/pyproject.toml#L183
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
5f38cb8
to
b3bed5f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ready to me, except for the 'test' subpackage.
Signed-off-by: Cristian Le <[email protected]>
any luck with this PR? |
description = "Run scripts with multiple Python versions" | ||
|
||
[[tool.hatch.envs.test.matrix]] | ||
python = ["3.9", "3.11", "3.12"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason the 3.10
is not in the matrix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for alignment with tmt
. Iiuc the python versions tested are those from Fedora and EPEL, but these are not much maintained, 3.13
is not there yet. We should make use of them though and then maybe we can revisit which python versions to track
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho 3.11 could be removed. 3.9, 3.12 and .13 when available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.13
should already be available, but let's keep it synced to tmt
first, and get it aligned with ruff
and other tooling as well. Then when we make changes of the infrastructure in tmt
we just open equivalents here.
Here are various modernizaitons to the pyproject
Switched toDropped as people are opposed to itsrc-layout
structure.bin/fmf
file to a script entry-pointRemovedSmall cleanups #237.travis.yaml
python-coveralls
dependency was removed. Not sure where that one was usedsdist
in PyPI #224