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

Make verification = '**.md' consistent across verification cases #165

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

simopier
Copy link
Collaborator

@simopier simopier commented Sep 8, 2024

(Ref. #12)

@simopier simopier self-assigned this Sep 8, 2024
@moosebuild
Copy link

Job Documentation on 77aa677 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link

Job Coverage on 77aa677 wanted to post the following:

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

cticenhour
cticenhour previously approved these changes Sep 10, 2024
@cticenhour
Copy link
Member

cticenhour commented Sep 10, 2024

Wait. The CI-generated VVR doesn't have these listed....I need to explore why before this can be merged. https://mooseframework.inl.gov/tmap8/docs/PRs/165/site/sqa/tmap8_vvr.html

@cticenhour cticenhour dismissed their stale review September 10, 2024 19:36

New information precludes approval at this time.

@cticenhour cticenhour self-assigned this Sep 10, 2024
@cticenhour cticenhour added the DO NOT MERGE Do not merge this PR until further notice label Sep 10, 2024
@simopier simopier marked this pull request as draft September 11, 2024 13:19
@simopier
Copy link
Collaborator Author

@cticenhour - just a gentle ping.

@cticenhour
Copy link
Member

Thanks for the reminder - I will get to this when I return next week. Maybe Thursday afternoon at the earliest.

Copy link
Member

@cticenhour cticenhour left a comment

Choose a reason for hiding this comment

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

Alright, I know what's going on here now. I am working on an issue for MOOSE so that I work on adjusting behavior in MooseDocs. In the meantime, here's what's up:

The verification and validation parameters need to go down into each test spec, individually, rather than live at the top-level. This is because, as currently designed, one verification document was assumed to pair with one test specification. Given we tend to have multiple tests correspond to a single verification case, this assumption is a poor one for our needs. I will fix it. In the meantime, to close this PR, please restore what was done in ver-1d and then emulate that in all other V&V tests. Then I can open a new PR at a later time to make this cleaner once my adjustments are merged into MOOSE.

EDIT: We have decided to wait until the fix is in MOOSE to approach this PR again.

@cticenhour
Copy link
Member

cticenhour commented Oct 23, 2024

MOOSE PR is idaholab/moose#28916

EDIT: Merged in just now (2024-Oct-23, 4:30PM MT), but will take a few hours before it makes it into the submodule. I will let you know when we can re-trigger testing.

@moosebuild
Copy link

Job Documentation on 77aa677 : invalidated by @simopier

Restarting this now After MOOSE's SQA update

@simopier
Copy link
Collaborator Author

@cticenhour
Now that idaholab/moose#28916 is merged, can that go in?

I just restarted the documentation tests.

@moosebuild
Copy link

moosebuild commented Oct 24, 2024

Job Documentation, step Sync to remote on 7ff5c44 wanted to post the following:

View the site here

This comment will be updated on new commits.

@cticenhour
Copy link
Member

This could be merged in, because it doesn't break anything, but won't take full effect until what is in moose next makes it to moose master. Next is blocked last I checked. ☹️

@moosebuild
Copy link

Job Documentation on 77aa677 : invalidated by @cticenhour

SQA changes now in MOOSE submodule in TMAP8

@cticenhour
Copy link
Member

@simopier I think you need to rebase on top of current devel to get this to work properly. An invalidation doesn't seem to be enough.

@simopier simopier marked this pull request as ready for review October 25, 2024 16:14
@moosebuild
Copy link

Job Coverage, step Generate coverage on 7ff5c44 wanted to post the following:

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

@cticenhour cticenhour removed the DO NOT MERGE Do not merge this PR until further notice label Oct 28, 2024
Copy link
Member

@cticenhour cticenhour left a comment

Choose a reason for hiding this comment

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

After the MOOSE fix, the VVR in the doco preview now shows all verification cases selected in this PR. Looks good!

@cticenhour cticenhour merged commit d0436fb into devel Oct 28, 2024
8 checks passed
@cticenhour cticenhour deleted the TMAP8_Verification_tag branch October 28, 2024 17:35
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.

3 participants