-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
sunkit-instruments affiliated package #335
Comments
@dstansby You up for doing this review? |
👍 |
NotesIntegrationI'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. DocsAPI 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. CommunityI 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. |
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). |
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:
|
👍 This probably applies to more than just sunkit-instruments. The file in the repo is a big 👎 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. |
Actually I am also 👎 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. |
I don't think we have really documented how exactly we handle responses to reviews, but given sunpy/sunkit-instruments#78 and the fact we do actually have a CoC, will you re-review? |
@dstansby Given the responses above, are you willing to re-review or does your current review still stand? |
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. |
v0.3.1 is now released if you could re-review that. |
Version reviewed: v0.3.1 |
Given the scores, |
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:
The text was updated successfully, but these errors were encountered: