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.

Sign up for GitHub

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

Jump to bottom

ENH: adding -R option #55

Merged
merged 3 commits into from
Sep 13, 2023
Merged

ENH: adding -R option #55

merged 3 commits into from
Sep 13, 2023

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Aug 23, 2023

To close #54

After this, it probably wouldn't hurt to tag a 0.11 release (maybe after doing a version audit to bump a few versions for the included plugins)

bsipocz added 3 commits August 22, 2023 20:58
@bsipocz
ENH: Adding -R as a short version for —remote-data
43900f1 
The option was available in astropy for a long time, and then was added to remotedata, but had to be removed due to a clash with pytest-leaks
@bsipocz
REL: limit remotedata version to avoid double definition of -R
898b2f8
@bsipocz
Adding changelog and docs
ef05ed4
@bsipocz bsipocz added this to the v0.11.0 milestone Aug 23, 2023
@pllim pllim requested a review from saimn August 23, 2023 15:28
@pllim
Copy link
Member

pllim commented Aug 23, 2023

xref astropy/pytest-remotedata#70

@pllim pllim mentioned this pull request Aug 23, 2023
Merged
@pllim
Copy link
Member

pllim commented Aug 23, 2023

I would like @saimn to review this if possible since he added some other flags here before. But if he is busy and this is time sensitive, feel free to ping me. Thanks!

@bsipocz
Copy link
Member Author

bsipocz commented Aug 23, 2023 •
edited
Loading

I would like @saimn to review this if possible since he added some other flags here before. But if he is busy and this is time sensitive, feel free to ping me. Thanks!

The technicalities are definitely correct, the question is whether this meta package is indeed the place for this flag. We had some discussions on it in several issues, and I'm happy with the compromise as it frees up pytest-remotedata for more generic usage (btw, I want to remove 'astropy' as a choice as well, but that's a separate ticket), while it keeps the convenience for those of using this option like forever in astropy land.

@bsipocz
Copy link
Member Author

bsipocz commented Sep 13, 2023

I've merged the other PR, can I get the approval on this to go ahead with the releases?

pllim
pllim approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Didn't hear any objection from Simon, so let's try this out. Thanks!

@pllim pllim merged commit 39e0bc4 into astropy:main Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pllim pllim pllim approved these changes

@saimn saimn Awaiting requested review from saimn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.11.0
Development

Successfully merging this pull request may close these issues.

Request: Move "-R" option from pytest-remotedata to wrapper pytest-astropy
2 participants
@bsipocz @pllim