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

Remove co2_containment code, has been moved to new repo ccs-scripts #633

Merged
merged 2 commits into from
Nov 24, 2023

Conversation

AudunSektnanNR
Copy link
Contributor

No description provided.

@AudunSektnanNR AudunSektnanNR marked this pull request as ready for review November 23, 2023 16:05
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (47f93ad) 84.70% compared to head (05e7ddd) 86.14%.

❗ Current head 05e7ddd differs from pull request most recent head 652b4c1. Consider uploading reports for the commit 652b4c1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #633      +/-   ##
==========================================
+ Coverage   84.70%   86.14%   +1.44%     
==========================================
  Files          52       49       -3     
  Lines        7510     7054     -456     
==========================================
- Hits         6361     6077     -284     
+ Misses       1149      977     -172     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mferrera mferrera self-requested a review November 24, 2023 07:02
@mferrera
Copy link
Collaborator

Will this script reach Komodo stable from ccs-scripts from the November release, or December release?

@AudunSektnanNR
Copy link
Contributor Author

It is not included in the November release, but will be included in the December release. Currently it is only on komodo bleeding.

We thought it would be best to remove the code from subscript in case it created some kind of conflict for the co2_containment code (subscript vs ccs-scripts)? Or is this unproblematic since it is from packages/repos with different names? Note that we haven't yet registered the code in ccs-scripts as plugin for ERT.

@mferrera
Copy link
Collaborator

If co2_containment as a command line invocation exists both here and in ccs-scripts in stable, then running co2_containment may call it from either here or ccs-scripts, but won't cause any problems otherwise. It depends mostly on the order in which Komodo installs it -- the latter one taking precedence.

So it makes a meaningful difference if co2_containment has been changed or updated in ccs-scripts, but otherwise doesn't really matter.

I guess, taking this in today, we could propose a second upgraded version for December. I think it should be okay.

@mferrera
Copy link
Collaborator

co2_containment ✅ ⛔️ ⛔️

Please also remove here and then I think it's good to go 😄

@AudunSektnanNR
Copy link
Contributor Author

If co2_containment as a command line invocation exists both here and in ccs-scripts in stable, then running co2_containment may call it from either here or ccs-scripts, but won't cause any problems otherwise. It depends mostly on the order in which Komodo installs it -- the latter one taking precedence.

So it makes a meaningful difference if co2_containment has been changed or updated in ccs-scripts, but otherwise doesn't really matter.

I guess, taking this in today, we could propose a second upgraded version for December. I think it should be okay.

Thanks for answer, yes there have been some new changes in ccs-scripts for co2_containment, so it will not be indentical to the version in subscript

Copy link
Collaborator

@mferrera mferrera left a comment

Choose a reason for hiding this comment

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

Unfortunately we've passed the freeze window now so these changes will come into Komodo stable in the January release.

@mferrera mferrera merged commit d0f04a1 into equinor:main Nov 24, 2023
4 checks passed
@AudunSektnanNR AudunSektnanNR deleted the br_as branch November 24, 2023 13:43
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