Skip to content

ENH: Add install-tags filtering for wheel generation (updated) #286

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

Closed
wants to merge 20 commits into from

Conversation

peter-urban
Copy link
Contributor

This is a PR for #68
It is a continuation of (and should probably replace) @Brishen's PR #267 which used my code, but has not been updated (since two weeks) to include the suggestions/changes made in reaction to @FFY00's comments.

@peter-urban peter-urban changed the title ENH: Add install-tags filtering for wheel generation (2) ENH: Add install-tags filtering for wheel generation (updated) Jan 31, 2023
@FFY00
Copy link
Member

FFY00 commented Jan 31, 2023

Hi @peter-urban, thank you! This is just missing a test, and applying the changes from pre-commit. Could you either tick the "allow maintainers to make changes" box in this PR, so that I can push that, or apply the pre-commit changes yourself and add the test below to test_project.toml. After that, we can merge 😊

def test_install_tags(package_purelib_and_platlib, tmp_path_session):
    project = mesonpy.Project(
        package_purelib_and_platlib,
        tmp_path_session,
        meson_args={
            'install': ['--tags', 'purelib'],
        }
    )
    assert project.is_pure

@peter-urban
Copy link
Contributor Author

peter-urban commented Jan 31, 2023

@FFY00 thanks :-) I merged with main again and added the test you suggested (I assume you meant test_project.py)
I wanted to give you write access, but for some reason I cannot see the "allow maintainers to make changes" box. (Maybe because I merge from a fork that belongs to an "organization" and not to my personal account) So I just hope the changes are ok now?

Looking forward to get this merged!

@FFY00
Copy link
Member

FFY00 commented Jan 31, 2023

Yeah 😅

Thanks! I think it's just missing the pre-commit changes. You can run pre-commit run --all-files and commit the changes.

@peter-urban
Copy link
Contributor Author

thanks, first time I work with pre-commit :-)

@FFY00
Copy link
Member

FFY00 commented Jan 31, 2023

No worries. Thank you for getting this through.

The Debian CI failure is unrelated. Let's just wait to see if the rest of the CI is green, and then we can merge 🚀

@FFY00
Copy link
Member

FFY00 commented Feb 1, 2023

Ugh, seems now we have conflicts. I am just gonna open a new PR and fix it myself, as I don't want to bother you.

@FFY00
Copy link
Member

FFY00 commented Feb 1, 2023

Closed in favor of #288. Thank you so much for the contribution @peter-urban!

@FFY00 FFY00 closed this Feb 1, 2023
@peter-urban
Copy link
Contributor Author

peter-urban commented Feb 1, 2023 via email

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