python-wheel-build / fromager

Build your own wheels
https://pypi.org/project/fromager/
Apache License 2.0
3 stars 9 forks source link

optimize sdist production during bootstrapping #173

Closed rd4398 closed 1 month ago

rd4398 commented 1 month ago

I added to check whether sdist is already built before. If I get a None, call to build_sdist()

  1. Should there be anything in the else case? (Reference to my latest comment on issue)
  2. Should I include a unit test for this?
dhellmann commented 1 month ago

A test would definitely be good. I'm not sure how easy it would be to create a unit test, but see what you can come up with. If that looks really complicated, maybe an e2e test would make more sense in this case? There are some other e2e tests that do things like run a build, remove some of the artifacts, then build again that could serve as a starting point for an example. The test can also do things like look for specific log messages indicating that fromager is building the sdist.

rd4398 commented 1 month ago

Okay, I will try to come up with a unit test first else e2e test if things get complicated. Thanks

rd4398 commented 1 month ago

Does this look like a good unit test?

rd4398 commented 1 month ago

I added an e2e test. Can you please check? Also, not sure why pkglint checks are failing

rd4398 commented 1 month ago

Fixed failing pkglintchecks

rd4398 commented 1 month ago

Changed error messages as per suggestion and used --log-file to generate log file Let me know if anymore changes are required!

rd4398 commented 1 month ago

Can you please approve and merge if everything looks fine?