sunpy / sunpy.org

The SunPy website
https://sunpy.org
Other
19 stars 35 forks source link

sunkit-instruments affiliated package #335

Closed nabobalis closed 1 year ago

nabobalis commented 2 years ago

Package Details

Description of Package

Stores instrument code that does not belong to a package that is maintained by the instrument teams.

Package Review

Editor Submission Checklist

Instructions to Reviewer

Please copy the following and select the ranking for each criteria, the full review criteria can be found here:

* Functionality           : General Package / Specialized Package / Not Relevant
* Integration             : Well integrated / Partially Integrated / Minimal Integration
* Documentation           : Extensive / Some / Little
* Testing                 : Excellent / Good / Needs Work
* Duplication             : None / Some / Major
* Community               : Excellent / Good / Needs Work
* Development Status      : Stable / Subject to Change / Low Activity / Needs Work
Cadair commented 2 years ago

@dstansby You up for doing this review?

dstansby commented 2 years ago

👍

dstansby commented 2 years ago

Notes

Integration

I've opened a bunch of issues on the repostory. Most of the integration improvements that could be made are returning astropy tables from functions where they currently return dicts. There's also a few areas where a standard time object or unit object should be accepted as input/output.

Docs

API docs are good (with some small gaps here or there). There is only a single example for the whole library. There are no guides or tutorials for each sub-package.

Community

I cannot find a code of conduct anywhere obvious, either in the repository or linked in the documentation. Otherwise all the other community criteria are met.

Any questions about my review let me know - if any of the above points are improved, I would be very happy to re-review.

Cadair commented 2 years ago

I cannot find a code of conduct anywhere obvious

I don't know how we make the fact clear that all sunpy maintained packages follow the sunpy CoC. The CoC is linked from the sidebar in the GH repo (as it inherits from the org).

dstansby commented 2 years ago

I've just found that, but given I was actively looking for a CoC and couldn't find one I think it needs to be more obvious. Places I looked were:

Cadair commented 2 years ago

:+1: This probably applies to more than just sunkit-instruments.

The file in the repo is a big :-1: from me, we do not want to be maintaining more than one copy of the CoC text. The canonical version is here: https://github.com/sunpy/.github/blob/main/CODE_OF_CONDUCT.md which is built into the website here: https://sunpy.org/coc we should link to that in the readme and docs for all packages of ours.

Cadair commented 2 years ago

Actually I am also :-1: on just throwing it in the docs given we already have it in the nav bar. We should probably add it to the theme in the footer as we have discussed elsewhere.

Cadair commented 2 years ago

I don't think we have really documented how exactly we handle responses to reviews, but given https://github.com/sunpy/sunkit-instruments/pull/78 and the fact we do actually have a CoC, will you re-review?

wtbarnes commented 2 years ago

@dstansby Given the responses above, are you willing to re-review or does your current review still stand?

dstansby commented 2 years ago

I forgot to put the verson reviewed in my original review, but I reviewed v0.3.0. It seems like there hasn't been an updated release with any changes, but if a new release is made I would be happy to re-review.

Cadair commented 2 years ago

v0.3.1 is now released if you could re-review that.

dstansby commented 2 years ago

Version reviewed: v0.3.1

wtbarnes commented 1 year ago

Given the scores, sunkit-instruments has passed the review and can now be added to the webpage as an affiliated package.