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
remove dead read/write StarList c++ tickets/dm-9416 #28
Conversation
3f8cfb3
to
b99a3ef
Compare
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 do, in fact, have changes to request, but they're straightforward enough to not require re-review.
include/lsst/jointcal/AstroUtils.h
Outdated
@@ -1,37 +1,15 @@ | |||
// This may look like C code, but it is really -*- C++ -*- | |||
#ifndef USNOUTILS__H | |||
#define USNOUTILS__H |
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 must be kept, but should be standardized to the LSST convention. Same with the #endif
down below, obviously.
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.
Done, and filed DM-9495 to fix the rest of them.
include/lsst/jointcal/AstroUtils.h
Outdated
@@ -1,37 +1,15 @@ | |||
// This may look like C code, but it is really -*- C++ -*- |
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.
Should be parseable by emacs.
include/lsst/jointcal/BaseStar.h
Outdated
/* what concerns the BaseStarList's : */ | ||
|
||
|
||
/* BaseStarList */ |
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.
Redundant and uninformative comment; remove.
include/lsst/jointcal/StarList.cc
Outdated
@@ -18,138 +18,6 @@ namespace lsst { | |||
namespace jointcal { |
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.
Nobody expects to include a .cc
file; consider either renaming (if it must be used as a header file, which I assume is the case) or moving it to src/
.
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.
Oh dear! I didn't even notice that this file was in include/
. Good catch!
Fixed starList templates to use explicit instantiation
No description provided.