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

Handle compatibility with dev version of AgentsExampleZoo better #956

Merged
merged 3 commits into from
Jan 16, 2024

Conversation

Tortar
Copy link
Member

@Tortar Tortar commented Jan 14, 2024

Since #875 has some problems I do this separately, see #875 (comment) for why we need this

edit: actually #875 is fine because also #946 has the same errors, but both work locally. However, let's merge this separately anyway since it is more sensible anyway

@Tortar Tortar requested a review from Datseris January 14, 2024 23:30
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (cbb8140) 85.71% compared to head (560ec35) 85.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #956   +/-   ##
=======================================
  Coverage   85.71%   85.71%           
=======================================
  Files          36       36           
  Lines        2542     2542           
=======================================
  Hits         2179     2179           
  Misses        363      363           

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

@Tortar
Copy link
Member Author

Tortar commented Jan 14, 2024

Strangely enough this works instead, github is crazy :S

Maybe it is that this is more recent pr (?), so merging the other two before this one will be enough to make CI happy, this is my bet :D

@Datseris
Copy link
Member

The existence of this PR highlights why one should avoid circular dependencies in packages.

@Tortar
Copy link
Member Author

Tortar commented Jan 15, 2024

I think that it actually higlights this when both packages depends on unreleased versions. We are in this strange situation because of this. When both packages will be released, there will be no problem like this anymore, it will be as if the models where inside Agents.jl. We will just pin the major version on both packages to ensure everything works.

I think the only problem which could arise is that if we make a major version of Agents without updating AgentsExampleZoo he will need a downgrade of Agents. But this will never happen if we don't completely forget about AgentsExampleZoo :D

@Datseris
Copy link
Member

I think that it actually higlights this when both packages depends on unreleased versions.

The same problem will also always happen for every major version change in either package.

@Datseris
Copy link
Member

In any case, let's just get this in for now and we can consider more how to deal with it before releasing v6.

@Datseris Datseris merged commit 5f6d6ef into main Jan 16, 2024
4 of 5 checks passed
@Datseris Datseris deleted the compat branch January 16, 2024 14:32
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