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

Patch to develop merge with fun reverts and rebases #1201

Merged
merged 40 commits into from
Aug 15, 2024

Conversation

brtietz
Copy link
Collaborator

@brtietz brtietz commented Aug 8, 2024

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.

sjanzou and others added 30 commits January 17, 2024 04:53
Tidal turbine power curve generation
Add drilling cost curves to geothermal cost cmod
Copy link
Collaborator

@mjprilliman mjprilliman left a comment

Choose a reason for hiding this comment

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

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).

@tyneises
Copy link
Collaborator

tyneises commented Aug 9, 2024

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?

@sjanzou
Copy link
Collaborator

sjanzou commented Aug 15, 2024

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!
image

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
image

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.

Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

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

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).

@tyneises
Copy link
Collaborator

Thanks @sjanzou ! Should we keep this pull request open and try to complete the merge here (after patch 2 is released)?

Copy link
Collaborator

@dguittet dguittet left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for handling this!

@brtietz brtietz merged commit 7fb465d into develop Aug 15, 2024
8 checks passed
@brtietz brtietz deleted the patch_merge_take_one_more branch December 13, 2024 19:21
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.

5 participants