Skip to content

Conversation

@nicoddemus
Copy link
Member

Initially we envisioned pixi.devenv.toml files dependencies to be included as:

includes = ["../core"]

However, in the end we went with a different syntax:

[devenv]
upstream = ["../core"]

Initially we envisioned `pixi.devenv.toml` files dependencies to be included as:

```toml
includes = ["../core"]
```

However, in the end we went with a different syntax:

```toml
[devenv]
upstream = ["../core"]
```
@nicoddemus nicoddemus requested review from a team, prusse-martin and tadeu and removed request for tadeu November 19, 2025 15:54
@nicoddemus nicoddemus merged commit 185eca1 into master Nov 19, 2025
3 checks passed
Comment on lines +109 to +117
match data:
case {"devenv": {"upstream": upstream}} if upstream:
return [
Path(os.path.abspath(dev_env_file.parent / get_relative_path(p)))
/ "pixi.devenv.toml"
for p in upstream
]
case _:
return []
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like to avoid those conditionals (case ... if ...:) in match/case constructs =)

>>> def what_is(v):
...   match v:
...     case [*a]:
...       print(f'{a=}')
...     case set(b):
...       print(f'{b=}')
...     case c:
...       print(f'{c=}')
...
>>> what_is([1,2,3])
a=[1, 2, 3]
>>> what_is((1,2,3))
a=[1, 2, 3]
>>> what_is({1,2,3})
b={1, 2, 3}
>>> what_is({1:11,2:22,3:33})
c={1: 11, 2: 22, 3: 33}
>>> what_is([])
a=[]
Suggested change
match data:
case {"devenv": {"upstream": upstream}} if upstream:
return [
Path(os.path.abspath(dev_env_file.parent / get_relative_path(p)))
/ "pixi.devenv.toml"
for p in upstream
]
case _:
return []
match data:
case {"devenv": {"upstream": [*upstream]}}:
return [
Path(os.path.abspath(dev_env_file.parent / get_relative_path(p)))
/ "pixi.devenv.toml"
for p in upstream
]
case _:
return []

Looks better to me.
And what if that upstream is bound to some other thing like a string?

Copy link
Contributor

@prusse-martin prusse-martin Nov 19, 2025

Choose a reason for hiding this comment

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

Maybe it could even be

Suggested change
match data:
case {"devenv": {"upstream": upstream}} if upstream:
return [
Path(os.path.abspath(dev_env_file.parent / get_relative_path(p)))
/ "pixi.devenv.toml"
for p in upstream
]
case _:
return []
match data:
case {"devenv": {"upstream": [*upstream]}}:
return [
(dev_env_file.parent / get_relative_path(p)).absolute()
/ "pixi.devenv.toml"
for p in upstream
]
case {"devenv": {"upstream": invalid_upstream}}:
raise RuntimeError(
f'Invalid upstream declaration: {invalid_upstream!r}\nThe expected value should be a list.'
)
case _:
return []

or even

Maybe it could even be

Suggested change
match data:
case {"devenv": {"upstream": upstream}} if upstream:
return [
Path(os.path.abspath(dev_env_file.parent / get_relative_path(p)))
/ "pixi.devenv.toml"
for p in upstream
]
case _:
return []
match data:
case {"devenv": {"upstream": [*upstream]}}:
return [
(dev_env_file.parent / get_relative_path(p)).absolute()
/ "pixi.devenv.toml"
for p in upstream
]
case {"devenv": {"upstream": str(upstream)}}:
return [
(
dev_env_file.parent / get_relative_path(upstream)
).absolute() / "pixi.devenv.toml"
]
case {"devenv": {"upstream": invalid_upstream}}:
raise RuntimeError(
f'Invalid upstream declaration: {invalid_upstream!r}\nThe expected value should be a list.'
)
case _:
return []

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is a matter of taste, and TBH my version looks better to me, so I will keep my version if you don't mind.

@nicoddemus nicoddemus deleted the adapt-pixi-release branch November 19, 2025 18:15
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.

4 participants