Conversation
setup.py
Outdated
| '': 'src', | ||
| }, | ||
| packages=find_packages( | ||
| where="./src", |
There was a problem hiding this comment.
I know this can be subjective, but would you consider to even simplify the structure a little bit more, by putting all the source code inside msal_extensions/* rather than src/msal_extensions/*? That is likely the popular way (see the top two results in searching Python package structure, this one and that one. Also, it is presumably the intended way, because it matches the default behavior of find_packages(where=...), so that you won't even need to write this line, nor the package_dir above.
There was a problem hiding this comment.
You make a strong argument :) This is fixed in 522be13
There was a problem hiding this comment.
Note: I realized, one of the reasons I had segmented this into src and tests was to intrinsically exclude tests from the packages that were discovered by setuptools. To combat this, I added 1a39ab7.
This flow is based on code found here: https://stackoverflow.com/questions/17583443/what-is-the-correct-way-to-share-package-version-with-setup-py-and-the-package/39671214#39671214 However the pattern was expanded slightly to handle String Literal Prefixes as defined here: https://docs.python.org/3/reference/lexical_analysis.html#grammar-token-stringprefix
setup.py
Outdated
| ], | ||
| extra_require={ | ||
| 'dev': [ | ||
| 'pytest', |
There was a problem hiding this comment.
@rayluo, mind confirming whether or not pytest is your preferred way to author/execute tests?
There was a problem hiding this comment.
I believe there is less a need to standardize the test framework, so I wasn't even paying much attention to this line. :-P You can choose whatever you want.
But then you kind of remind me, this pytest line would better NOT be put inside extraS_require (you had a typo there, by the way). The extras_require are mean to organize extra FUNCTIONALITY, explained here.
For test dependency, they can be put inside a different parameter, tests_require, so it would look like this: tests_require=['pytest']. For example, this is what PyJWT did.
|
Also, I've just created a new |
This prevents "find_packages" from picking it up, even if there is no exclusion set.
rayluo
left a comment
rayluo
left a comment
There was a problem hiding this comment.
It looks shiny and clean! Yay! 🚢
2 participants
This PR seeks to add the most basic skeleton of a Python project. This should ensure that before I begin submitting the other changes from my fork, the basic structure of the project is okay.