-
Notifications
You must be signed in to change notification settings - Fork 28
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
Cleanup manifest #38
Cleanup manifest #38
Conversation
Codecov Report
@@ Coverage Diff @@
## main #38 +/- ##
==========================================
+ Coverage 87.53% 87.82% +0.28%
==========================================
Files 22 22
Lines 1051 1043 -8
==========================================
- Hits 920 916 -4
+ Misses 131 127 -4
Continue to review full report at Codecov.
|
npe2/_command_registry.py
Outdated
# TODO: validate argumemnts and type constraints | ||
# possibly wrap command in a type validator? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favor of handling this w #29 right now; the cli tool can do some initial validation to type check.
# When the list of extensions for the writer doesn't match the | ||
# extension in the filename, keep searching. | ||
|
||
# Nothing got found |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this fixes some broken tests I was getting on the napari side. napari's CI misses these at the moment because npe2 isn't installed when the test suite is running.
description="The name of the plugin. Should correspond to the python " | ||
"package name for this plugin.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, i didn't particularly mind most of the comments that you removed here. I think many of them are still "open questions" that haven't received any particularly special attention that warrants comment removal. or are perhaps just generally useful in terms of where the code was inspired (such as the vscode comments). |
Co-authored-by: Talley Lambert <[email protected]>
Co-authored-by: Talley Lambert <[email protected]>
Yes. Let me know if I missed anything. I'm keeping this in sync w #40 |
why are you removing license (like other fields, it could certainly be auto-populated by package metadata... but does it need to be removed?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outside of license, this is fine with me. Can we leave that discussion for another PR?
I think we generally need to consider the process of taking data from setup.cfg a little better. Let's keep it independent of "cleanup"
added back.
agreed. I also like the idea of separating that discussion from this issue. |
closes #17
This removes several fields. These include
license(keep)I also went through and tried to remove comments, TODO's, add field documentation, etc.
The EXAMPLE folder was removed; it contained an old example.
Please note TODO's and FIXME's that you'd like to keep.