Patch to develop merge with fun reverts and rebases#1201
Conversation
injection drilling costs
… getem-cost-updates
Tidal turbine power curve generation
Getem cost updates
Add drilling cost curves to geothermal cost cmod
mjprilliman
left a comment
There was a problem hiding this comment.
Marine and Geothermal changes look good and shouldn't cause any issues. Need to confirm geothermal tests still pass once the project builds (build errors coming from csp cmods and solver core).
|
Brian's merge didn't fix the merge issues with the CSP files. I replaced the poorly merged version of these files on this pull request with the Develop version. These files were not updated recently on patch with the exception of @dguittet 's commit to align cmod group names 0b50eaf . We need to reapply these changes in Develop. There are also small differences between Patch and Develop in ssc/common.cpp. I don't know if the merge got this right. @sjanzou can you please check if this file is correct in this pull request? |
@tyneises , Good catch! common.cpp has been updated in patch and the comparison with develop shows that the changes are not there (patch on left and develop on right) and common.cpp is not in this pull request! It seems like some changes are not captured in this pull request - here is a list of differences from patch (left) and develop (right) as of 8/14/2024 All the files in this pull request look good to me; however, I think we need to do another pull request after patch 2 is released. Several of the merges for patch 2 are not included in this pull request as shown above. |
There was a problem hiding this comment.
The file changes in this pull request look fine; however, there are several changes in patch that are not in develop that are not included in this pull request - see my previous comment for additional file changes (files in red text indicate later change dates).
|
Thanks @sjanzou ! Should we keep this pull request open and try to complete the merge here (after patch 2 is released)? |
dguittet
left a comment
There was a problem hiding this comment.
LGTM, thanks for handling this!
See email thread "patch to develop merge conflicts." This is the least-bad approach I've seen from automated methods, but if we think this is bad we can throw it out and ask someone to do it manually.
Please confirm that the patch to develop changes are correct and push additional manual fixes to this branch as needed. The tests are failing at the moment, so this is definitely needed.
Please wait to merge until all 4 reviewers have reviewed, since all of you have code that might have been affected by this process.