Skip to content

MANIFEST.in: Include missing test-related files#18

Merged
chaudum merged 2 commits intochaudum:masterfrom
smcv:include-test-data
May 25, 2020
Merged

MANIFEST.in: Include missing test-related files#18
chaudum merged 2 commits intochaudum:masterfrom
smcv:include-test-data

Conversation

@smcv
Copy link
Contributor

@smcv smcv commented May 14, 2020

Otherwise, the tests will fail when run in a PyPI source release.

@smcv
Copy link
Contributor Author

smcv commented May 15, 2020

CI will fail until rebased on #20.

It would probably be useful if one of the CI steps did a setup.py sdist, unpacked it and ran tests on it, similar to Autotools make distcheck and Meson ninja dist, but I haven't implemented that yet.

@chaudum
Copy link
Owner

chaudum commented May 20, 2020

Would you mind adding a changelog entry that the distribution now also contains the CHANGES.md, and test data?

smcv added 2 commits May 21, 2020 09:08
Otherwise, the tests will fail when run in a PyPI source release.

Signed-off-by: Simon McVittie <smcv@debian.org>
Signed-off-by: Simon McVittie <smcv@debian.org>
@smcv smcv force-pushed the include-test-data branch from cb405b7 to 0578054 Compare May 21, 2020 08:11
@smcv
Copy link
Contributor Author

smcv commented May 21, 2020

Would you mind adding a changelog entry that the distribution now also contains the CHANGES.md, and test data?

Done, for now, but I think it's usually better for a maintainer to update the changelog after branches get merged, or before the release. That's what I do in projects I maintain (for example, see NEWS in https://gitlab.freedesktop.org/dbus/dbus).

If multiple merge requests contain user-facing changes that deserve changelog entries, and the changelog entry is included in the merge request itself, then that guarantees they will have git conflicts, which is particularly problematic if it isn't clear which one will get merged first.

@smcv
Copy link
Contributor Author

smcv commented May 21, 2020

It would probably be useful if one of the CI steps did a setup.py sdist, unpacked it and ran tests on it, similar to Autotools make distcheck and Meson ninja dist, but I haven't implemented that yet.

That's implemented in #21.

@chaudum
Copy link
Owner

chaudum commented May 21, 2020

Would you mind adding a changelog entry that the distribution now also contains the CHANGES.md, and test data?

Done, for now, but I think it's usually better for a maintainer to update the changelog after branches get merged, or before the release. That's what I do in projects I maintain (for example, see NEWS in https://gitlab.freedesktop.org/dbus/dbus).

Thanks!

With regards to the changelog. Personally, I think a changelog entry belongs to the commit where the change happens. That way, the git history of the CHANGES file automatically references the changes.

Adding the changelog later, can cause items to be forgotten about. Additionally you'd always need to reference the commit in the changelog so you can reenact where the changes happened. This could be done by automatically generating the changelog for a release. There are several tools for that. Usually they require to write the commit messages in a certain way so they could be "parsed" and identified as e.g. fixes, features, etc. This is something I could imaging using.

If multiple merge requests contain user-facing changes that deserve changelog entries, and the changelog entry is included in the merge request itself, then that guarantees they will have git conflicts, which is particularly problematic if it isn't clear which one will get merged first.

That is true, but very likely only the case in much larger projects.


Btw, I am very glad about your contributions, since I've never worked on a project before that ends up as an official operating system package. :)

@smcv
Copy link
Contributor Author

smcv commented May 24, 2020

Is there anything else I need to do for this branch to be mergeable?

With regards to the changelog. Personally, I think a changelog entry belongs to the commit where the change happens.

I think we'll have to agree to disagree on this. I'll try to remember to include changelog entries in any further user-facing changes I contribute.

@chaudum chaudum merged commit 5fe6ac0 into chaudum:master May 25, 2020
@chaudum
Copy link
Owner

chaudum commented May 25, 2020

Is there anything else I need to do for this branch to be mergeable?

No. Merged now.

I think we'll have to agree to disagree on this. I'll try to remember to include changelog entries in any further user-facing changes I contribute.

Totally fine :-)

@smcv smcv deleted the include-test-data branch May 25, 2020 18:39
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

Successfully merging this pull request may close these issues.

2 participants