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

Replace old GENERAL_OBSERVATION error message #2369

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

BjarneHerland
Copy link
Contributor

Issue
Resolves #938. As a bonus, the method conf_instance_has_path_error() is neither used not defined anywhere so declaration is removed from conf.hpp

Approach
Replaced static text to dynamically report keywords for which files cannot be located, topped off with an extensive unit-test. A static (internal) method is used in conf.cpp, utilizing std::set to accumulate errors while avoiding duplicates. Keywords are separated by newline and returned as char * since this method is exposed to C.

@BjarneHerland
Copy link
Contributor Author

test ert please

@BjarneHerland BjarneHerland force-pushed the fix-observations-errormsg branch 2 times, most recently from 0458c45 to 90a4d71 Compare November 19, 2021 13:27
libres/lib/config/conf.cpp Outdated Show resolved Hide resolved
libres/tests/enkf/enkf_obs_paths_detailed.cpp Outdated Show resolved Hide resolved
libres/tests/CMakeLists.txt Show resolved Hide resolved
libres/tests/enkf/enkf_obs_paths_detailed.cpp Outdated Show resolved Hide resolved
@dafeda
Copy link
Contributor

dafeda commented Nov 25, 2021

Now that the new tmpdir is merged, should we try using it here?

I've with help from @eivindjahren made use of it here: #2425

@BjarneHerland
Copy link
Contributor Author

test ert please

Now that the new tmpdir is merged, should we try using it here?

I've with help from @eivindjahren made use of it here: #2425

Working on it... 😄


/*
* Set up workarea, create and write start of file.
* Returns pointer to workares since it needs to be kept alive
Copy link
Contributor

Choose a reason for hiding this comment

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

Type in "Returns pointer to workares since it needs to be kept alive", should be workarea

* for the actual test
*/
void setup() {
util_make_path("obs_path");
Copy link
Contributor

Choose a reason for hiding this comment

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

Use filesystem::create_directory instead?

@dafeda dafeda self-requested a review November 25, 2021 12:11
@BjarneHerland
Copy link
Contributor Author

test ert please

Copy link
Contributor

@eivindjahren eivindjahren left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@BjarneHerland
Copy link
Contributor Author

test ert please

3 similar comments
@BjarneHerland
Copy link
Contributor Author

test ert please

@BjarneHerland
Copy link
Contributor Author

test ert please

@BjarneHerland
Copy link
Contributor Author

test ert please

Replaced to dynamically report keywords for which files cannot be
located, topped off with an extensive unit-test. The method
conf_instance_has_path_error() is neither used not defined anywhere so
declaration is removed from conf.hpp
@BjarneHerland BjarneHerland merged commit a313f10 into equinor:main Nov 26, 2021
@BjarneHerland BjarneHerland deleted the fix-observations-errormsg branch January 26, 2022 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update GENERAL_OBSERVATION error message
4 participants