moose icon indicating copy to clipboard operation
moose copied to clipboard

Issue conversion confusion between THM and sockeye origin during the merge of idaholab/THM into the thermal hydraulics module

Open GiudGiud opened this issue 2 years ago • 14 comments

Bug Description

In the thermal hydraulics module, several commits are referencing sockeye issues well into the 19000s for the issue number. These issues do not exist. What does exist are THM issues (in the hundreds) that were migrated to MOOSE, and have issue number in the 19000s.

These commits were referencing sockeye issues and the issue number referenced got modified by the migration script to point to the MOOSE issues created for the THM issues with the same numbers (as the sockeye issues).

The correct thing to do would have been to reference in the commit a MOOSE issue created for the relevant sockeye issue

This is a separate issue than #26336. For context, that one concerns:

  • tests that were originally in RELAP7 and were moved into THM
  • tests in THM that referenced issues in RELAP7 For both of these, the issue referenced is in relap7, so not public, so instead of porting the issue to moose, issue #26336 was created and used instead.

Steps to Reproduce

Check commits that reference sockeye issues in THM

Impact

Commits are referencing unrelated THM-migrated issues

GiudGiud avatar Dec 23 '23 22:12 GiudGiud

For fixing this, one obvious solution would be to rewrite MOOSE history and change these commits. They could point either to a dedicated sockeye issues issue, or ported sockeye issues in the MOOSE repo. This is pretty disruptive repo-wide, so I ll only do it if I have strong support from the team.

If not, I'm open to other ideas.

GiudGiud avatar Dec 29 '23 10:12 GiudGiud

This sounds like a really big git rebase -i; it would need to respect merges. I don't know what other option there is other than

  • Adding an additional issue description to the currently unrelated MOOSE THM issue such that the referenced ticket number represents both a THM issue and a sockeye issue
  • Lettting the commits continue to point to a wrong issue number. This is probably bad for SQA

Seems like something @permcody and @cticenhour should weigh in on

lindsayad avatar Dec 29 '23 18:12 lindsayad

I am not in favor of a history rewrite at this point for the reasons already stated, and we cannot let commits point to an issue number that has nothing to do with the commit. That's easy audit fodder to me. I think @lindsayad's first bullet is a good way:

Adding an additional issue description to the currently unrelated MOOSE THM issue such that the referenced ticket number represents both a THM issue and a sockeye issue

If we wanted to reduce textual clutter and potential confusion in a single issue, a slight tweak to this might be the creation of ported Sockeye issues and then link them through the commit-referenced THM issue (making the passthrough obvious in markdown formatting). This maintains linkability from commit to ticket and ticket searchability with less confusion, I think. Open to discussion on the finer points with this idea.

cticenhour avatar Jan 02 '24 17:01 cticenhour

What I like about the "2 issue in one issue" solution is that if it's not enough, we can go back and do the other solution easily. Once we have re-written history, the barrier to changing our mind about the solution is much higher.

I m not sure I should be the assignee here? This is more sockeye-ey work

GiudGiud avatar Jan 02 '24 22:01 GiudGiud

@loganharbour I don't know who performed the migration (you or David?), but if there's some kind of migration script, we need to modify it for the future.

joshuahansel avatar Jan 04 '24 14:01 joshuahansel

I don't understand the proposed solution. It sound like something like Refs idaholab/sockeye#100 got changed to Refs idaholab/sockeye#19000, not Refs #19000. Take for example MOOSE commit 48db61307ed87b58a96e944215f13378138cf7bc. It has Refs idaholab/sockeye#19845. We would need to create a Sockeye issue 19845, right, which we probably can't do?

joshuahansel avatar Jan 04 '24 14:01 joshuahansel

There's some kind of SQA procedure for referencing the incorrect issue number, right? For instance if someone accidentally references the wrong issue number on a commit today, there should be a way to fix it without rewriting MOOSE history. I say "should" not as in I know the SQA procedures but as in a sensible world. We should be able to fix/document mistakes when they happen, right? If not, I think we need to work on SQA policies 😛

joshuahansel avatar Jan 04 '24 14:01 joshuahansel

I believe @permcody made the conversation. We only converted THM issues to MOOSE issues, not sockeye issues. It looks like the rewrite for issue # got applied to messages with other repos and probably shouldn't have.

loganharbour avatar Jan 04 '24 14:01 loganharbour

We would need to create a Sockeye issue 19845, right, which we probably can't do?

you could either create 18,000 dummy issues to get to 19,000 or maybe just add the 19,000 number in the issue title? so the issue would be renamed? "Something something #19445 #243"

GiudGiud avatar Jan 05 '24 10:01 GiudGiud

@permcody @cticenhour Is there any SQA procedure for addressing mistakes? Like maybe keeping a list of log revisions somewhere?

joshuahansel avatar Jan 05 '24 13:01 joshuahansel

I don't believe we have a procedure for something quite like this. Making a log revision page attached to our SQA documentation (probably a link off of the RTM?) might be the "easiest" way to record this for potential auditors, but I'll let @permcody chime in too.

cticenhour avatar Jan 05 '24 15:01 cticenhour

As long as we document it. We can do whatever we want to fix the problem. Auditors won’t punish us for mistakes if there is documentation explaining what’s going on.

On Fri, Jan 5, 2024 at 8:55 AM Casey Icenhour @.***> wrote:

I don't believe we have a procedure for something quite like this. Making a log revision page attached to our SQA documentation (probably a link off of the RTM?) might be the "easiest" way to record this for potential auditors, but I'll let @permcody https://github.com/permcody chime in too.

— Reply to this email directly, view it on GitHub https://github.com/idaholab/moose/issues/26411#issuecomment-1878887675, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXFOIDOYBWWHEYGAR45XUDYNAO5VAVCNFSM6AAAAABBBCUO3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZYHA4DONRXGU . You are receiving this because you were mentioned.Message ID: @.***>

permcody avatar Jan 06 '24 02:01 permcody

@cticenhour Can you start this commit log revision page, and then afterward I can fill it in with all of the Sockeye ones?

joshuahansel avatar Jan 08 '24 13:01 joshuahansel

I will add it to my list and ping you when I have the PR queued up.

cticenhour avatar Jan 08 '24 15:01 cticenhour