revert mistaken merge in tasks_lfric_atm.cylc - #417
Conversation
|
Tom Hill (@tom-j-h) please may you cross check this change and compare to the intent for #163 I think that the re-insertion of the configuration for C896 was not related to your change, and essentially backed out part of #214 Please may you share your opinion on this? |
iboutle
left a comment
There was a problem hiding this comment.
I concur with your analysis that this was a mistake in the merge - this is much easier to do now because github doesn't automatically resolve the changes within file when there aren't conflicts like fcm did, but relies on the author to select each difference by hand, which can easily result in the wrong one being selected!
|
Yes, looks like I made a mistake merging |
|
I'm happy this passes science review |
Pierre Siddall (Pierre-siddall)
left a comment
There was a problem hiding this comment.
Thanks Mark as this is a relatively trivial change which removes a task and it's associated configuration from the lfric_atm suite, this looks good to head into testing to me :) .
3c15cab
into
MetOffice:main
PR Summary
Sci/Tech Reviewer: iboutle
Code Reviewer: Pierre Siddall (@Pierre-siddall)
The merged change #214 altered the setup of the
lfric_atmtask dict, but part of that change was reverted by #163this looks like a merge error, a chunk of code was added, and a chunk of code removed by #214 next door was re-inserted.
perhaps a conflict resolution error?
this change restores the configuration of C896 tests to the state as mandated by #214
Code Quality Checklist
Testing
trac.log
Test Suite Results - lfric_apps - fix_lfric_atm_tasks/run1
Suite Information
Task Information
✅ succeeded tasks - 1165
Security Considerations
Performance Impact
AI Assistance and Attribution
Documentation
PSyclone Approval
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review