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

Adding functor input capability to ParsedAux #27959

Merged
merged 7 commits into from
Jun 27, 2024

Conversation

kyleeswanson
Copy link

@kyleeswanson kyleeswanson commented Jun 20, 2024

Reason

This PR supports functor input capability for ParsedAux class - contributes to Issue #21244

Copy link
Contributor

@pbehne pbehne left a comment

Choose a reason for hiding this comment

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

Overall: great job!

While my requested changes may look like a lot, you have already done the most important work: adding new capability. My small suggestions are aimed at making sure that we are following MOOSE's coding standards and trying to make this Auxkernel idiot-proof at the input file level.

General comments:

  1. I don't see a reference to the issue that this PR requests. I think it is related to Uniformize parsed capabilities #21244. If a related issue does not exist, you need to create one and reference it.
  2. Please edit the description of the PR (you left it blank) to describe what the PR does. Reference the issue it addresses.
  3. In order for your PR to merge, it needs to pass CIVET. One of the things that CIVET requires is that at least one of your commit messages should reference the relevant issue number (i.e., Addressed code review suggestions related to issue #21244.). Note the hashtag is crucial for CIVET to interpret the number as a reference.

Let me know if you have any questions or are ready for a re-review!

framework/include/auxkernels/ParsedAux.h Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/doc/content/source/auxkernels/ParsedAux.md Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/include/auxkernels/ParsedAux.h Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Show resolved Hide resolved
@GiudGiud GiudGiud self-assigned this Jun 23, 2024
Copy link
Contributor

@pbehne pbehne left a comment

Choose a reason for hiding this comment

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

Just a few more nitpicky requests.

framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Outdated Show resolved Hide resolved
test/tests/auxkernels/parsed_aux/tests Show resolved Hide resolved
framework/src/auxkernels/ParsedAux.C Outdated Show resolved Hide resolved
Issue idaholab#21244

Update framework/src/auxkernels/ParsedAux.C

Co-authored-by: Patrick Behne <[email protected]>

Apply suggestions from code review

Co-authored-by: Patrick Behne <[email protected]>

Fixed clang-tidy and formatting of ParsedAux.C
Issue idaholab#21244
@GiudGiud
Copy link
Contributor

@pbehne

@moosebuild
Copy link
Contributor

moosebuild commented Jun 26, 2024

Job Documentation on b8ac89d wanted to post the following:

View the site here

This comment will be updated on new commits.

@idaholab idaholab deleted a comment from moosebuild Jun 26, 2024
@GiudGiud GiudGiud requested a review from pbehne June 27, 2024 02:14
@moosebuild
Copy link
Contributor

Job Modules parallel on ba44786 : invalidated by @kyleeswanson

Timed out

Copy link
Contributor

@pbehne pbehne left a comment

Choose a reason for hiding this comment

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

Looks good! Great job, @kyleeswanson!

@moosebuild
Copy link
Contributor

Job Coverage on b8ac89d wanted to post the following:

Framework coverage

62dc98 #27959 b8ac89
Total Total +/- New
Rate 85.06% 85.07% +0.00% 96.15%
Hits 104412 104454 +42 50
Misses 18335 18336 +1 2

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud GiudGiud merged commit 1b53c96 into idaholab:next Jun 27, 2024
47 checks passed
@GiudGiud
Copy link
Contributor

Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants