Skip to content

Remove astropy_helpers and update packaging to use recommendations in APE 17#3598

Merged
nabobalis merged 10 commits intosunpy:masterfrom
Cadair:extension_helpers
Jan 2, 2020
Merged

Remove astropy_helpers and update packaging to use recommendations in APE 17#3598
nabobalis merged 10 commits intosunpy:masterfrom
Cadair:extension_helpers

Conversation

@Cadair
Copy link
Member

@Cadair Cadair commented Dec 11, 2019 •
edited
Loading

This PR contains a few packaging changes to follow the work being done in APE 17

The only noticeable change here is that python setup.py test and python setup.py build_docs are no longer supported. If you still run those commands the alternatives are printed, with links to the documentation.

I will hold off on merging this, so that a) maintainers see it and know about the changes to the setup commands and b) wait until extension helpers has a chance to mature a little.

@ghost
Copy link

ghost commented Dec 11, 2019 •
edited by ghost
Loading

Thanks for the pull request @Cadair! Everything looks great!

@Cadair Cadair added this to the 2.0 milestone Dec 11, 2019
astrofrog
astrofrog reviewed Dec 11, 2019
View reviewed changes
@Cadair Cadair force-pushed the extension_helpers branch 2 times, most recently from b58333a to 1638e60 Compare December 11, 2019 19:25
@Cadair Cadair added the No Changelog Entry Needed Skip all changelog checks. label Dec 12, 2019
dstansby
dstansby previously requested changes Dec 12, 2019
View reviewed changes
Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

Would be nice to add a quick changelog to say that the python setup.py ... options no longer work.

@Cadair
Copy link
Member Author

Cadair commented Dec 12, 2019

This is an interesting meta question about the changelog. Is the changelog just for users or also for developers? Personally, I think this is an infrastructure change which shouldn't go in the changelog.

I will be sending info about this around to sunpy-dev before we merge it though.

@dstansby
Copy link
Contributor

dstansby commented Dec 12, 2019

Isn't there a dev or similar category in the changelog types?

@Cadair
Copy link
Member Author

Cadair commented Dec 12, 2019

There is an internal one, I suppose it could go there. I still think that there is a policy decision to be made here (but this probably isn't the place).

@ayshih
Copy link
Member

ayshih commented Dec 13, 2019

I'd like to see make html mentioned as an option for building the docs (speaking as someone who keeps forgetting whether tox actually works on his setup).

I support adding a changelog entry for this. There may be some users using the python setup.py ... commands, e.g., to make sure that their setup isn't borked or to have docs available offline.

@Cadair Cadair removed the No Changelog Entry Needed Skip all changelog checks. label Dec 13, 2019
@ghost
Copy link

ghost commented Dec 13, 2019 •
edited by ghost
Loading

Thanks for the pull request @Cadair! Everything looks great!

nabobalis
nabobalis reviewed Dec 13, 2019
View reviewed changes
@nabobalis nabobalis force-pushed the extension_helpers branch 5 times, most recently from 8d7387a to 2c3b9b4 Compare December 15, 2019 12:02
@nabobalis nabobalis marked this pull request as ready for review December 15, 2019 12:03
@nabobalis
Copy link
Member

nabobalis commented Dec 15, 2019

I added a changelog and added astropyfrog's suggestion.

@nabobalis nabobalis added Needs Review Needs review(s) before merge. Infrastructure Issues or PRs that affect the CI or packaging. labels Dec 15, 2019
@nabobalis
Copy link
Member

nabobalis commented Dec 15, 2019

Do we want to backport this to 1.1?

nabobalis
nabobalis approved these changes Dec 15, 2019
View reviewed changes
Copy link
Member

@nabobalis nabobalis left a comment

Choose a reason for hiding this comment

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

Unless there is more to do? We can merge this. (Its similar to what I've done in sunkit-image).

@Cadair
Copy link
Member Author

Cadair commented Dec 15, 2019

The main thing I want to wait for here is a non-pre release of extension-helpers.

@dstansby dstansby self-requested a review December 15, 2019 15:45
@nabobalis
Copy link
Member

nabobalis commented Dec 21, 2019

Seems an API change from extension_helpers came.

Cadair
Cadair commented Dec 23, 2019
View reviewed changes
@Cadair
Copy link
Member Author

Cadair commented Dec 23, 2019

Given the release of extension helpers, I think this is good for merging once approved.

@Cadair Cadair dismissed dstansby’s stale review December 23, 2019 11:38

Comment addressed.

@nabobalis nabobalis merged commit 53addaa into sunpy:master Jan 2, 2020
@Cadair Cadair deleted the extension_helpers branch January 3, 2020 10:26
@abhijeetmanhas abhijeetmanhas mentioned this pull request Jan 10, 2020
Closed
@dstansby dstansby removed the Needs Review Needs review(s) before merge. label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@astrofrog astrofrog astrofrog left review comments

@nabobalis nabobalis nabobalis approved these changes

@dstansby dstansby dstansby approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Infrastructure Issues or PRs that affect the CI or packaging.

Projects

None yet

Milestone

2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@Cadair @dstansby @ayshih @nabobalis @astrofrog