Skip to content
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

DM-33613: modernize with clang tidy, fix obvious code defects #215

Merged
merged 1 commit into from Apr 1, 2022

Conversation

mwittgen
Copy link
Contributor

No description provided.

Copy link
Collaborator

@parejkoj parejkoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handful of comments, otherwise this looks like an improvement.

lsst::jointcal::JointcalControl const &control);

/**
* Add a pre-constructed ccdImage to the ccdImageList.
*/
void addCcdImage(std::shared_ptr<CcdImage> const ccdImage) { ccdImageList.push_back(ccdImage); }
void addCcdImage(std::shared_ptr<CcdImage> const& ccdImage) { ccdImageList.push_back(ccdImage); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should be const std::...<>& ccdImage?

std::string const &filter, std::shared_ptr<afw::image::PhotoCalib> photoCalib,
CcdImage(afw::table::SourceCatalog &record, const std::shared_ptr<lsst::afw::geom::SkyWcs>& wcs,
const std::shared_ptr<lsst::afw::image::VisitInfo>& visitInfo, lsst::geom::Box2I const &bbox,
std::string filter, std::shared_ptr<afw::image::PhotoCalib> photoCalib,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we can also make the photoCalib pointer const & while we're at it.

include/lsst/jointcal/StarMatch.h Outdated Show resolved Hide resolved
@mwittgen mwittgen merged commit f4254d1 into main Apr 1, 2022
@mwittgen mwittgen deleted the tickets/DM-33613 branch April 1, 2022 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants