opm-common icon indicating copy to clipboard operation
opm-common copied to clipboard

Update RPTSCHED mnemonics instead of replace (support WELLS=N)

Open vkip opened this issue 1 year ago • 1 comments

vkip avatar Sep 23 '24 12:09 vkip

jenkins build this opm-simulators=5626 please

vkip avatar Sep 23 '24 12:09 vkip

This looks fine from a quick glance, but I'll just ask a general question before I do a more thorough review: While there is an API change here, it does not appear that the downstream PR (OPM/opm-simulators#5626) depends on that API change. Do we need to review/merge the two PRs at the same time or can we review this PR–in opm-common–first and then review its companion PR once this has been merged?

bska avatar Sep 24 '24 08:09 bska

This looks fine from a quick glance, but I'll just ask a general question before I do a more thorough review: While there is an API change here, it does not appear that the downstream PR (OPM/opm-simulators#5626) depends on that API change. Do we need to review/merge the two PRs at the same time or can we review this PR–in opm-common–first and then review its companion PR once this has been merged?

I think only the const 'at' function is required in simulators from the one is common, so doing common first should be fine.

vkip avatar Sep 24 '24 09:09 vkip

Thanks! Will keep in mind for next time.

vkip avatar Sep 24 '24 10:09 vkip