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 CONTAINER keyword #3834

Merged
merged 1 commit into from
Sep 5, 2022

Conversation

frode-aarstad
Copy link
Contributor

@frode-aarstad frode-aarstad commented Aug 29, 2022

Issue
Resolves #3747

Approach
Short description of the approach

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

@frode-aarstad frode-aarstad marked this pull request as draft August 29, 2022 12:33
@frode-aarstad frode-aarstad force-pushed the remove-container-type branch 3 times, most recently from dfa32ce to eed0292 Compare August 30, 2022 07:25
@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2022

Codecov Report

Merging #3834 (c17f6ba) into main (f7f669c) will increase coverage by 0.20%.
The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main    #3834      +/-   ##
==========================================
+ Coverage   63.85%   64.06%   +0.20%     
==========================================
  Files         595      593       -2     
  Lines       44580    44378     -202     
  Branches     4012     3977      -35     
==========================================
- Hits        28467    28431      -36     
+ Misses      14873    14719     -154     
+ Partials     1240     1228      -12     
Impacted Files Coverage Δ
src/clib/lib/enkf/config_keys.cpp 0.84% <ø> (+<0.01%) ⬆️
src/clib/lib/enkf/enkf_state.cpp 22.48% <ø> (-0.37%) ⬇️
src/clib/lib/enkf/ensemble_config.cpp 38.11% <ø> (+1.02%) ⬆️
...rc/ert/_c_wrappers/enkf/config/enkf_config_node.py 75.90% <ø> (-1.08%) ⬇️
src/ert/_c_wrappers/enkf/data/enkf_node.py 86.95% <ø> (-0.15%) ⬇️
src/ert/_c_wrappers/enkf/ensemble_config.py 90.98% <ø> (-0.43%) ⬇️
...c/ert/_c_wrappers/enkf/enums/ert_impl_type_enum.py 100.00% <ø> (ø)
src/clib/lib/enkf/enkf_node.cpp 41.95% <25.00%> (+4.00%) ⬆️
src/clib/lib/enkf/enkf_config_node.cpp 51.31% <100.00%> (+3.92%) ⬆️
src/clib/lib/enkf/enkf_state_nodes.cpp 57.14% <100.00%> (+7.14%) ⬆️
... and 10 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@frode-aarstad frode-aarstad marked this pull request as ready for review August 30, 2022 12:42
@dafeda
Copy link
Contributor

dafeda commented Aug 30, 2022

Did a quick search for CONTAINER in your branch and got two hits, on in config_keys.hpp and one in enkf_types.hpp.
I think we can get rid of those as well.

@frode-aarstad frode-aarstad added the release-notes:maintenance Automatically categorise as maintenance change in release notes label Aug 31, 2022
@frode-aarstad frode-aarstad self-assigned this Aug 31, 2022
@oyvindeide
Copy link
Collaborator

Did you check if the failing workflow is related to the changes in this PR?

@frode-aarstad
Copy link
Contributor Author

Did you check if the failing workflow is related to the changes in this PR?

I believe they are historical problems. Seems like clang-tidy runs on the files touched in the PR and reports old issues. Can have a look and see if they are easily solvable

Copy link
Collaborator

@oyvindeide oyvindeide left a comment

Choose a reason for hiding this comment

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

LGTM! Good job cleaning! The file causing the annotation to fail will be removed in #3843 so no need to do anything about it in this PR.

@frode-aarstad frode-aarstad changed the title Remove container type Remove CONTAINER keyword Sep 5, 2022
@frode-aarstad frode-aarstad force-pushed the remove-container-type branch 2 times, most recently from 496e026 to 3bd5518 Compare September 5, 2022 11:41
@frode-aarstad frode-aarstad merged commit fa519e7 into equinor:main Sep 5, 2022
@frode-aarstad frode-aarstad deleted the remove-container-type branch September 5, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes:maintenance Automatically categorise as maintenance change in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove CONTAINER type
4 participants