Skip to content
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

New build system #241

Closed
N-Coder opened this issue Apr 11, 2020 · 1 comment
Closed

New build system #241

N-Coder opened this issue Apr 11, 2020 · 1 comment

Comments

@N-Coder
Copy link
Member

N-Coder commented Apr 11, 2020

See #243 for an updated version.

For my current PR #240 I'm adding hypothesis as a test dependency to allow generation of random input strings to better test parsing and escaping logic (see e44e4b3). I also changed how we load the grammar file, to make the package zip safe again (see 3cafe53). While working on how these could be integrated into our testsuite, I found out that setup.py test is deprecated (1 2). The recommendation was to use tox to test the generated package directly using pytest instead of relying on setup.py to test the sources. Additionally, there's a designated successor to setup.py and setup.cfg (in which we probably accumulated a few outdated definitions), the build-system independent pyproject.toml. Additionally, there are newer tools like pipenv, flit and poetry to help with the whole build and publishing process. As I was not quite happy with how the whole development process of ics.py was set up, I wanted to give them a shot. Collecting some opinion around the internet, it seemed that flit was mostly targeted at very simple low-configuration projects and the development of pipenv somehow stagnated, while poetry seemed to be a well suited solution.

My draft with all ics.py sources moved according to the new recommended format can be found here:
https://github.com/N-Coder/ics.py-poetry
The first of the two main configuration files is now pyproject.toml, where all meta-information on the project and how it can be installed (i.e. dependencies) and built are stored (without the need to actually execute the file and have some specific setuptools lying around). The second is tox.ini, where all testing related functionality is configured. A third file .bumpversion.cfg is from a small tool that helps with updating the version number in all places when doing a new release. The poetry.lock file optionally stores the dependency versions against which we want to develop, which is independent from the versions the library pulls in as dependency itself, where we are pretty liberal, and the versions we test against, which is always the latest releases. All library sourcecode now resides within a src folder, which is recommended as it prevents you from accidentally having the sources in your PATH when you want to test the final package.

The root directory now looks very clean and all those files have their specific purpose. If you want to configure how testing is done, you find all information in tox.ini. If you want to run the tests (i.e. pytest, doctest, flake8, mypy and check that the docs build), simply run tox - you don't have to worry about which versions in which venvs are installed and whether you're directly testing against the sources or against a built package, tox handles all that for you. This not only comes in very handy when running the tests manually, but should also ensure that CI does exactly the same. On a side note, we're now again publishing coverage data.

If you just want to run the tests and don't need to fiddle around with the development version of ics in an interactive shell, that's all you need. For the fiddling part, just run poetry install and you will have a turnkey development environment set up. Use poetry shell or poetry run to easily access the venv poetry set up for you. Publishing is now also very simple: just call poetry publish --build and the tool will take care of the rest. This made it very easy to make some releases on the testing pypi instance.

The third and last tool you might want is bumpversion, if you are making new releases. But there is no need anymore to handle any venvs yourself or to install all ics.py dependencies globally. To summarize, if you want to hit the ground running and publish a new release on a newly set-up machine, the following should suffice:

git clone https://github.com/N-Coder/ics.py-poetry.git && cd ics.py-poetry
pip install tox poetry bumpversion --user
tox # make sure all the test run
bumpversion --verbose release # 0.8.0-dev -> 0.8.0 (release)
poetry build # build the package
tox --recreate # ensure that the version numbers are consistent
# check changelog and amend if necessary
git push && git push --tags
poetry publish # publish to pypi
bumpversion --verbose minor # 0.8.0 (release) -> 0.9.0-dev
git push && git push --tags

You can try that out if you want -- except for the publishing part maybe. Also note that bumpversion directly makes a commit with the new version if you don't pass --no-commit or --dry-run, but that's no problem as you can easily amend any changes you want to make, e.g. to the changelog.

As these changes are partially based upon #240 but are also quite fundamental, I wanted to collect feedback first before making a second PR on top of the other or directly including the changes into #240. The only other thing #240 is still lacking is more testing (only few files already have close to 100% coverage), and I'd prefer to provide that using this new environment. So that's also some kind of cyclic dependency.

@N-Coder
Copy link
Member Author

N-Coder commented Apr 11, 2020

You can see the changes made here: new-parser-impl...new-build-sys
(if you're visiting from the future, b9e6ab9...281c9f6 is what they currently point to)

It's mostly the config files in the root directory that changed, but I also removed the dev directory as it should no longer be needed. Next to all files from the ./ics/ directory remain unchanged and are simply moved to ./src/ics/. I didn't copy the tests over yet, as I plan to rewrite most of them in my other branch. Maybe I should've just opened a new-parser-impl <- new-build-sys PR instead of this issue, then we could discuss the details there and once we've settled onto something we can squash the changes and merge into #240, where I can continue to work on the tests... 🤷‍♂️

@N-Coder N-Coder closed this as completed Apr 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant