Skip to content

introducing own .pre-commit-config.yaml for OPM-common #4791

Merged
GitPaean merged 5 commits into
OPM:masterfrom
GitPaean:opm-common-own-pre-commit
Oct 14, 2025
Merged

introducing own .pre-commit-config.yaml for OPM-common #4791
GitPaean merged 5 commits into
OPM:masterfrom
GitPaean:opm-common-own-pre-commit

Conversation

@GitPaean
Copy link
Copy Markdown
Member

And run pre-commit run --all-files to remove the white spaces and fixing the EOL at the end of all the files.

As discussed in #4790 , external folder is excluded.

There will be a few commits to fix the failures, mostly due to string processing and usage of strings.

@GitPaean GitPaean added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Oct 14, 2025
@GitPaean
Copy link
Copy Markdown
Member Author

jenkins build this please

f = open("opm/material/densead/EvaluationSpecializations.hpp", "w")
f.write(fileContents)
f.close()
# adding a newline at the end of the file if not present
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not totally sure why this is needed only for the file EvaluationSpecializations.hpp, while not other generated files.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not totally sure why this is needed only for the file EvaluationSpecializations.hpp, while not other generated files.

I don't know either. Someone more familiar with the templating system will have to give a more definitive answer.

@GitPaean
Copy link
Copy Markdown
Member Author

jenkins build this please

@GitPaean GitPaean force-pushed the opm-common-own-pre-commit branch from bb00eba to 230aaf1 Compare October 14, 2025 12:43
@GitPaean
Copy link
Copy Markdown
Member Author

GitPaean commented Oct 14, 2025

test_ERsm.cpp is using lines with pure white spaces and was difficult (for me) to fix the test after removing the white spaces. So test_ERsm.cpp is removed from the pre-commit whitespaces checking list .

now I see the comment line
// Dont touch the string literals - it is all part of the format; down to the number of whitespace ... :-(

@GitPaean
Copy link
Copy Markdown
Member Author

jenkins build this please

@GitPaean
Copy link
Copy Markdown
Member Author

Now the jenkins passed. https://ci.opm-project.org/job/opm-common-PR-builder/8952/

I need to split the last commit, just posting the build link for reference.

for white space removal for now. The tests utilizing lines with pure whitespaces
and difficult to fix.
since \n was added at the beginning of the messages, so it will not
generate the first line in the message with a white space.
@GitPaean GitPaean force-pushed the opm-common-own-pre-commit branch from 230aaf1 to 53e4dcb Compare October 14, 2025 13:26
@GitPaean
Copy link
Copy Markdown
Member Author

jenkins build this please

@GitPaean GitPaean marked this pull request as ready for review October 14, 2025 13:44
Copy link
Copy Markdown
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the updates. I think this looks good now. I don't quite understand why we need a mostly full copy of the NORNE_ATW2013 simulation model in our Python tree, but I'm not going to debate that here. That's for someone else to pick up later.

As for the whitespace changes to EvaluationSpecializations.hpp that escapes me too. Anyone want to chime in?

Comment on lines +98 to +99
// \n is added at the beginning of the messages through locationStringLine()
return fmt::format(R"(Problem with keywords{}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair enough. This could possibly be phrased more succinctly using fmt::join(), but I'm not going to go through that exercise now.

f = open("opm/material/densead/EvaluationSpecializations.hpp", "w")
f.write(fileContents)
f.close()
# adding a newline at the end of the file if not present
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not totally sure why this is needed only for the file EvaluationSpecializations.hpp, while not other generated files.

I don't know either. Someone more familiar with the templating system will have to give a more definitive answer.

@GitPaean
Copy link
Copy Markdown
Member Author

Thanks for the approval. I am getting it in now.

@GitPaean GitPaean merged commit 7d8d6e2 into OPM:master Oct 14, 2025
2 checks passed
@GitPaean GitPaean deleted the opm-common-own-pre-commit branch October 15, 2025 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants