Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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](pytest-dev/pytest#5534) [2](https://tox.readthedocs.io/en/latest/example/basic.html#integration-with-setup-py-test-command)). 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. So this PR contains my attempt at migrating to `poetry`, with all ics.py sources moved according to the new recommended format. 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. 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`](https://github.com/N-Coder/ics.py-poetry/blob/master/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](https://github.com/N-Coder/ics.py-poetry/blob/master/.github/workflows/pythonpackage.yml) does exactly the same. On a side note, we're now again publishing [coverage data](https://codecov.io/gh/N-Coder/ics.py-poetry). 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`](https://python-poetry.org/docs/cli/#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](https://test.pypi.org/project/ics/#history). 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: ```bash 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. The above information on developing, testing and publishing can now also be found in the docs (see CONTRIBUTING.rst). As these changes are partially based upon #240 but are also quite fundamental, I wanted to collect feedback first before 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 `tox` in this new environment. So that's also some kind of cyclic dependency. Sorry for the (now superfluous) issue I opened before. So @C4ptainCrunch (and maybe also @aureooms and @tomschr), what's your opinion on this? * migrate repo structure to poetry * fix src path for pytest * add doc skeleton * implement handling of attachments * import project files * set version * fix sphinx build with poetry * don't use poetry within tox see python-poetry/poetry#1941 (comment) * fix timezone tests * change coveralls action * try codecov * bugfixes * add bumpversion * separate src inspection (flake8+mypy src/) from package testing (pytest tests/) to fix PATH problems * bugfixes * Merge branch 'master' into new-parser-impl * remove old files * add dev and publish instructions * checker happiness `noqa` and `type: ignore` are now only used for actual bugs in the checkers unfortunately, current pyflakes dislikes `type: ignore[something]`, so we can't ignore specific mypy bugs until pyflakes 2.2 is in flakes8 * more checker happiness * Apply suggestions from code review Co-Authored-By: Tom Schraitle <tomschr@users.noreply.github.com> * use gitignore directly from github instead of gitignore.io * Apply suggestions from code review to tox.ini * fix tox.ini * add pypy support Mostly by moving/splitting test dependencies to different sections in tox.ini as mypy and pypy don't work well together and it is sufficient to run mypy checks on CPython. * update developing documentation * fix non-ASCII whitespace handling * update test/dev dependencies
- Loading branch information
0b028d3
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.
@N-Coder Thanks Nico for all your efforts! π
May I mention a small aspect? I'm sure you are already aware of that, but GitHub has a bad habit: when you have an open pull request, whatever you write in the description or in your commits, it adds to the final commit message. In this case, the commit message is... ehm... huge. π
As commits should be brief and a summary of the most relevant points, a huge commit message would defeat this idea. Probably, it's a bit overwhelming for every developer who tries to understand what's going on. If something really needs to explained in more depth, we can add it to our (developer) documentation. All is needed to redact and strip down the final commit message to its bare minimum before merging it to master. π
Sorry, I don't want to give you a bad feeling. :-) Quite the opposite! IMHO a good project contains not only good code or good documentation, but also good commit messages. Thanks. :-)
0b028d3
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.
All good, no offense taken. π
I actually didn't question the behaviour of GitHub, but your point seems to be very valid.
Now that Nikita merged this into master, I can unfortunately no longer rewrite the messages and force-push them to my PR.
Maybe I could create a new branch where I rewrite these two (and maybe also older overlong) commit messages, if @C4ptainCrunch is okay with then force-pushing these to master.
0b028d3
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.
@N-Coder Thanks Nico. IMHO we don't need to force-push. Let use the commit message as it is and leave it as a (bad?) example for a better future. π