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

Rename a few objects based on issues created #27908

Merged
merged 8 commits into from
Jun 17, 2024

Conversation

GiudGiud
Copy link
Contributor

@GiudGiud GiudGiud self-assigned this Jun 16, 2024
@idaholab idaholab deleted a comment from moosebuild Jun 16, 2024
@GiudGiud GiudGiud force-pushed the PR_rename_a_few branch 3 times, most recently from 8538390 to e817871 Compare June 16, 2024 20:14
@idaholab idaholab deleted a comment from moosebuild Jun 16, 2024
@moosebuild
Copy link
Contributor

moosebuild commented Jun 16, 2024

Job Documentation on 4107b2e wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Jun 17, 2024

Job Coverage on 4107b2e wanted to post the following:

Framework coverage

ca913d #27908 4107b2
Total Total +/- New
Rate 85.08% 85.08% - 96.72%
Hits 104117 104117 - 236
Misses 18265 18265 - 8

Diff coverage report

Full coverage report

Modules coverage

Rdg

ca913d #27908 4107b2
Total Total +/- New
Rate 63.32% 63.32% - 95.65%
Hits 454 454 - 22
Misses 263 263 - 1

Diff coverage report

Full coverage report

Solid mechanics

ca913d #27908 4107b2
Total Total +/- New
Rate 84.86% 84.86% - 90.59%
Hits 27679 27679 - 77
Misses 4938 4938 - 8

Diff coverage report

Full coverage report

Full coverage reports

Reports

This comment will be updated on new commits.

@GiudGiud GiudGiud changed the title Rename a few objects based on issues created, delete one Rename a few objects based on issues created Jun 17, 2024
@GiudGiud GiudGiud marked this pull request as ready for review June 17, 2024 03:56
@GiudGiud
Copy link
Contributor Author

Should be ready for review now.

Copy link
Contributor

@joshuahansel joshuahansel left a comment

Choose a reason for hiding this comment

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

The renames seem good other than maybe ExamplePatchMeshGenerator - that one does sound like it needs to be moved to test (as suggested in the issue) if it has a name like that. And I would usually do "Test" instead of "Example", but this is an optional change.

@GiudGiud
Copy link
Contributor Author

GiudGiud commented Jun 17, 2024

It's heavily relied on in Bison so we cant move it to test.
We could move it to Bison

The rename solves my problem with it. (which is that it pretends to be a grouper/patcher of MGs)

@joshuahansel
Copy link
Contributor

joshuahansel commented Jun 17, 2024

It's relied upon in Bison for actual usage or just testing? If actual usage, then I guess "Example" is inappropriate. If just testing, we just need to have Bison add moose test objects in its test executable, right?

@GiudGiud
Copy link
Contributor Author

Just testing. That's not really Makefile work I would like to do.
The renaming is necessary even if we move it to test and do the Makefile work btw

@joshuahansel
Copy link
Contributor

Makefile work? I thought would just be a one-liner in BisonTestApp. For example, in SockeyeTestApp::registerAll() we do

ThermalHydraulicsTestApp::registerAll(f, af, s, true);

to get THM test objects. If that's not the case, then nevermind.

Copy link
Contributor

@joshuahansel joshuahansel left a comment

Choose a reason for hiding this comment

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

The suggested move of ExamplePatchMeshGenerator to test/ is an optional follow-up change (after Bison changes).

@GiudGiud
Copy link
Contributor Author

Thanks!
Ah yes it could be just that. That would be fine as long as Bison folks are fine with it

@GiudGiud GiudGiud merged commit 4db7fd7 into idaholab:next Jun 17, 2024
47 checks passed
@GiudGiud GiudGiud deleted the PR_rename_a_few branch June 17, 2024 17:18
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

3 participants