Skip to content

BUG: fix issues with wheel tags, and add more tests to cover tags#143

Merged
rgommers merged 8 commits intomainfrom
fix-tag
Sep 27, 2022
Merged

BUG: fix issues with wheel tags, and add more tests to cover tags#143
rgommers merged 8 commits intomainfrom
fix-tag

Conversation

@FFY00
Copy link
Member

@FFY00 FFY00 commented Sep 14, 2022

Closes gh-141

@FFY00 FFY00 changed the title TST: add test for wheel tags Add tests for tags and fix them Sep 14, 2022
@FFY00 FFY00 marked this pull request as ready for review September 14, 2022 18:09
@FFY00 FFY00 requested a review from rgommers September 14, 2022 18:09
@FFY00 FFY00 force-pushed the fix-tag branch 3 times, most recently from e71ab51 to f9814c5 Compare September 14, 2022 21:23
Copy link
Contributor

@rgommers rgommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @FFY00. This looks like an improvement, it seems to fix things in the test case I just submitted in combination with Meson master.

One change is needed, to not error out on unknown platforms.

@rgommers
Copy link
Contributor

Also, CI seems to be unhappy.

@rgommers rgommers added the bug Something isn't working label Sep 16, 2022
@rgommers rgommers added this to the v0.9.0 milestone Sep 27, 2022
FFY00 and others added 6 commits September 27, 2022 21:50
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
Signed-off-by: Filipe Laíns <lains@riseup.net>
@rgommers
Copy link
Contributor

rgommers commented Sep 27, 2022

More Mypy buggy-ness:

mesonpy/__init__.py:839: error: Returning Any from function declared to return "bool"  [no-any-return]

This works fine locally on both Linux and macOS, and the code is correct - just a little complicated with property decorators:(

The code itself should be fine, but the fixture is having
a hard to diagnose issue in CI in the setup phase.
@rgommers
Copy link
Contributor

More Mypy buggy-ness:

It depends on the exact Mypy version and the minor version of Python ... sigh. On Python 3.9 this error disappears, and another one appears. Guess we have to live with it, Mypy isn't going to mature any time soon.

@rgommers
Copy link
Contributor

This was all green modulo the mypy issue, the last commit should fix that.

@rgommers rgommers changed the title Add tests for tags and fix them BUG: fix issues with wheel tags, and add more tests to cover tags Sep 27, 2022
@rgommers
Copy link
Contributor

This is happy, in it goes. Thanks @FFY00!

@rgommers rgommers merged commit 2197935 into main Sep 27, 2022
@rgommers rgommers deleted the fix-tag branch September 27, 2022 21:53
@FFY00
Copy link
Member Author

FFY00 commented Sep 28, 2022

Thanks for all the work on getting this PR ready for merging. For next time, I'd prefer PRs that do multiple things to be rebase-merged, instead of squashed, the rule I try to abide by is one commit per logical change. In case you are not used to this kind of workflow, I recommend using fixup commits if you need to modify commits, as explained in https://openinput.readthedocs.io/en/latest/contributing/index.html#workflow. Thanks 😊

@rgommers
Copy link
Contributor

Sure, that is a good idea in most cases. I am familiar with it, will aim to rebase-merge where possible.

In cases where it would break git bisect it may be a bit questionable though, then it's more of a judgement call and squash-merge or a merge commit may be more helpful.

@FFY00
Copy link
Member Author

FFY00 commented Sep 28, 2022

Thanks! All commits being self-contained is also something I strive to achieve, so in this workflow you would apply the fixes needed to the PR to each respective commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wheel ABI tag heuristics producing py3-none again on Arch Linux

2 participants