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

Add ignored test case for conflicting deps in unrelated ws members #1007

Closed
wants to merge 2 commits into from

Conversation

maciektr
Copy link
Contributor

@maciektr maciektr commented Dec 19, 2023

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

@maciektr maciektr marked this pull request as ready for review December 19, 2023 21:31
@szymmis
Copy link
Contributor

szymmis commented Dec 20, 2023

What are those tests related to, especially in the context that they are ignored?

@maciektr
Copy link
Contributor Author

@szymmis basically bugs that we should sort out eventually

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

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

I do understand the reasoning. I am afraid how this will play out with PubGrub, as I believe the canonical algorithm will attempt to unify some_dep in this scenario.

Did some user request this behaviour? Do we need to commit to it?

@maciektr
Copy link
Contributor Author

maciektr commented Jan 2, 2024

Did some user request this behaviour? Do we need to commit to it?

Nope, just a thought experiment

I am afraid how this will play out with PubGrub

Well, I do not know as well. Though it seems that having same dep with different versions may actually make sense even in cairo - when the graph is disconnected.

@mkaput
Copy link
Member

mkaput commented Jan 3, 2024

Though it seems that having same dep with different versions may actually make sense even in cairo - when the graph is disconnected.

That's a highly philosophical argument. One could want instead to use a single version of dep throughout the workspace because it's well -- a workspace. Given a workspace of multiple contracts, I'm not sure the author will want the contract A to have dependency v0, while contract B -- v1.

I think we have to ask users on Telegram.

@maciektr maciektr force-pushed the spr/main/70c8f66b branch 2 times, most recently from 912ed7b to c68fb93 Compare January 3, 2024 12:38
@maciektr maciektr changed the base branch from main to spr/main/f67e9fd8 January 3, 2024 12:38
@maciektr maciektr marked this pull request as draft January 3, 2024 12:42
@maciektr maciektr changed the base branch from spr/main/f67e9fd8 to spr/main/001f9da5 January 3, 2024 13:14
github-merge-queue bot pushed a commit that referenced this pull request Jan 12, 2024
commit-id:001f9da5

---

**Stack**:
- #1007
- #1030⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do
not merge manually using the UI - doing so may have unexpected results.*
Base automatically changed from spr/main/001f9da5 to main January 12, 2024 10:41
@maciektr maciektr closed this Jan 17, 2024
@maciektr maciektr deleted the spr/main/70c8f66b branch February 8, 2024 10:31
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