-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Avoid duplicate code in astropy.vo.samp
#3612
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
Avoid duplicate code in astropy.vo.samp
#3612
Conversation
|
@anizami - thanks for the contribution, and sorry for not identifying the issues sooner! After some local testing, I was able to find out what was causing the issue. |
|
@mdboom - could you review the addition of the configuration item and let me know if this is correct? |
|
Marked this as "affects release" due to the new config item if nothing else. I personally have no problem adding a new config option in a patch release, but if you disagree we can change the milestone to 1.1. |
|
@embray - fine by me - thought just to check, I thought that 'affects-release' just meant that for bugs it was a problem with a released version not just dev? (didn't think we used that for non-bugs?) |
|
Yes -- the new ConfigurationItem definition looks fine. |
|
It's used for enhancements too--any user-visible change between releases. |
|
You can read it equivalently to "This needs a changelog entry". that's one of the main things I use it to check. |
… code from _call_, _notify_ and _reply_
7a03730 to
d92544f
Compare
|
Damn, I think I must have lost the configuration changes when rebasing... |
|
Recovered thanks to Dropbox. Version control redundancy is good. |
|
So's |
Avoid duplicate code in ``astropy.vo.samp``
|
@embray - thanks, learned something today! |
Avoid duplicate code in ``astropy.vo.samp``
Replaces #2219
Fixes #2029