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
[c++] Final rename SOMA_READER
-> SOMA_ARRAY_READER
#1201
Conversation
3f8a642
to
08b379e
Compare
Codecov ReportPatch coverage has no change and project coverage change:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #1201 +/- ##
===========================================
- Coverage 64.96% 50.84% -14.13%
===========================================
Files 91 61 -30
Lines 6737 4380 -2357
===========================================
- Hits 4377 2227 -2150
+ Misses 2360 2153 -207
Flags with carried forward coverage won't be shown. Click here to find out more. see 30 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
@@ -374,4 +374,4 @@ class SOMAArrayReader { | |||
|
|||
} // namespace tiledbsoma | |||
|
|||
#endif | |||
#endif // SOMA_ARRAY_READER |
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.
idk how people feel about this comment after #endif
but it's a long-held personal preference of mine ...
https://github.com/johnkerl/scripts/blame/main/fundam/hdrtop
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.
@eddelbuettel sorry to be cryptic.
Just an 11-year-old script which shows that every time I create a header file I want to do :.!hdrtop %
in vim
which makes
#ifndef FOO_H
#define FOO_H
#endif // FOO_H
showing that I've had this stylistic preference for long enough to have automated around it. :)
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.
Superflouous at the end of a file like this that has one and only exactly one header guard.
But in principle I like comments at the end of long and nested 'end' or closing brackets.
Lastly for the case of header guards: the cool kids switched to #pragma once
to not have this problem
Very lastly too minor an issue to worry. But if you feel strongly about it and have the urge to touch umpteen files go for it.
@eddelbuettel == 🚢 ? |
Yes I learned about here years ago. Apparently it is a thing: https://www.google.com/search?q=shipit+squirrel+meaning&oq=shipit+squirrel |
Sorry to be cryptic -- I should have said: given that you did , can you accept the PR? |
Thanks @eddelbuettel !! |
Yepper! Sorry about that -- was long day and I meant to 'shipit comment and approve_ and only managed the first part. Now fixed! Thanks for cleaning up in any and all dark corners. |
One more following on #1191