-
Notifications
You must be signed in to change notification settings - Fork 55
simplify core API #178
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
simplify core API #178
Conversation
Signed-off-by: Anton Dukhovnikov <[email protected]>
include/rawtoaces/rawtoaces_core.h
Outdated
/// The constructor. Takes the database search path as an optional | ||
/// parameter. | ||
/// @param search_path optional database search path. | ||
SpectralSolver( const std::vector<std::string> &search_path = {} ); |
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.
Can we make it search_paths
since it's a list of things?
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.
tricky, I'd rather call it search_directories then. usually $PATH is singular, but contains multiple locations
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
|
||
SpectralSolver::SpectralSolver() | ||
SpectralSolver::SpectralSolver( const std::vector<std::string> &search_path ) | ||
: _search_path( search_path ) |
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.
don't forget about this one to be _search_paths
as well if you happy with above suggestion
std::filesystem::path type_path( directory ); | ||
type_path.append( type ); | ||
if ( std::filesystem::exists( type_path ) ) | ||
{ |
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.
do we care to warn about something in else case of this and previous if?
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
Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
Co-authored-by: Aleksandr Motsjonov <[email protected]> Signed-off-by: Anton Dukhovnikov <[email protected]>
7b3a170
into
AcademySoftwareFoundation:master
* simplify core API Signed-off-by: Anton Dukhovnikov <[email protected]> * address review issues Signed-off-by: Anton Dukhovnikov <[email protected]> * address review issues Signed-off-by: Anton Dukhovnikov <[email protected]> * Update src/rawtoaces_core/rawtoaces_core.cpp Co-authored-by: Aleksandr Motsjonov <[email protected]> Signed-off-by: Anton Dukhovnikov <[email protected]> --------- Signed-off-by: Anton Dukhovnikov <[email protected]> Co-authored-by: Aleksandr Motsjonov <[email protected]> Signed-off-by: Savitha M <[email protected]>
No description provided.