Print

Print


@matthewfeickert commented on this pull request.

Awesome @amadio! I just scanned this now, so this isn't really a review, but just some things that hit me right away.


In bindings/python/project.toml:

> +[build-system]
+requires = ["setuptools", "wheel"]

Shouldn't this be pyproject.toml and not project.toml? Or are you doing something different that I don't know about?

⬇️ Suggested change
-[build-system]
-requires = ["setuptools", "wheel"]
+[build-system]
+requires = ["setuptools>=61.0"]
+build-backend = "setuptools.build_meta"

If you are using a pyproject.toml based system in keeping with PEP 518 then build-backend should be included. In https://scikit-hep.org/developer/pep621#pyprojecttoml-build-system we recommend a modern

[build-system]
requires = ["setuptools>=61.0"]
build-backend = "setuptools.build_meta"

but maybe there are limitations on this version of setuptools for XRootD?


In project.toml:

> @@ -0,0 +1,2 @@
+[build-system]

Same comments as above. Though if it is possible to get rid of the duplication of toml files in this PR that's an even better win.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <xrootd/xrootd/pull/2025/review/1462949827@github.com>

[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/xrootd/xrootd/pull/2025#pullrequestreview-1462949827", "url": "https://github.com/xrootd/xrootd/pull/2025#pullrequestreview-1462949827", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1