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

Fix imports for configs #529

Merged
merged 1 commit into from
Jul 14, 2023
Merged

Fix imports for configs #529

merged 1 commit into from
Jul 14, 2023

Conversation

frode-aarstad
Copy link
Contributor

No description provided.

@frode-aarstad frode-aarstad self-assigned this Jul 13, 2023
Copy link

@valentin-krasontovitsch valentin-krasontovitsch left a comment

Choose a reason for hiding this comment

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

i don't know if we have a system for dependency management that we use, but i feel like this is an API change of ert, and hence we should bump the version of ert as a dependency when updating these import lines - which of course requires merging ert first, and making a new tag / release, and us having ert as a versioned requirement in this project...
yes i see we have ert with a min version in setup.py
i suggest we wait for the ert PR to be merged, make a new tag, and then bump the min version here before merging

@frode-aarstad
Copy link
Contributor Author

i don't know if we have a system for dependency management that we use, but i feel like this is an API change of ert, and hence we should bump the version of ert as a dependency when updating these import lines - which of course requires merging ert first, and making a new tag / release, and us having ert as a versioned requirement in this project... yes i see we have ert with a min version in setup.py i suggest we wait for the ert PR to be merged, make a new tag, and then bump the min version here before merging

Not sure there is a formal process around this until we propose releases for komodo

@valentin-krasontovitsch

in my time as a release manager, i did not see this mentioned or formalized anywhere. but i guess it doesn't need to be formal - would you agree with the following steps:

  • merge the ert PR
  • make a new ert tag
  • bump the >= requirement of ert to that new tag as a part of this PR ?

@valentin-krasontovitsch
Copy link

valentin-krasontovitsch commented Jul 14, 2023

the test is failing with

E ModuleNotFoundError: No module named 'ert.config'

from the logs, we can see it installs

ert==5.1.0b3

so it looks like we have to bump the dependencies before the tests can pass - i guess this is a good way of formalizing it 😅

@oysteoh
Copy link
Contributor

oysteoh commented Jul 14, 2023

I guess a new tag on ert will do it as semeio then downloads the latest ?

@frode-aarstad frode-aarstad merged commit 04e313c into main Jul 14, 2023
7 checks passed
@frode-aarstad frode-aarstad deleted the move-parameter-config branch July 14, 2023 08:51
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