Conversation
Signed-off-by: Pradyot Ranjan <[email protected]>
prady0t
changed the title
lucascolley
changed the title
|
Hi @prady0t, thank you for the PR. I was testing/playing with it a little more. I added |
ruben-arts
left a comment
ruben-arts
left a comment
There was a problem hiding this comment.
I'm approving it from my side.
Yes, it looks much clearer now. Thanks for the changes. |
prady0t
left a comment
prady0t
left a comment
There was a problem hiding this comment.
@ruben-arts I think this is ready to be merged.
crates/pixi_cli/src/list.rs
Outdated
| pub fn license(&self) -> Cow<'_, str> { | ||
| match self { | ||
| Self::Conda(value) => value.record().license.as_deref().unwrap_or("").into(), | ||
| Self::PyPI(_value, _) => String::from("").into() |
There was a problem hiding this comment.
Can't find a way to extract PyPI license info. Maybe we need to add a field in PyPiPackageData in rattler? @nichmor
There was a problem hiding this comment.
Still not sure if we need to/can address this in rattler.
| } | ||
| } | ||
|
|
||
| pub fn license(&self) -> Cow<'_, str> { |
There was a problem hiding this comment.
Lets return Option<&str> here instead
| let license = if show_license { | ||
| Some(package.license().into_owned()) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
I think it makes more sense to just remove this argument and return Option<..> from package.license instead.
| # Test that --show-license flag shows the License column header | ||
| verify_cli_command( | ||
| [pixi, "list", "--manifest-path", manifest_path, "--show-license"], | ||
| stdout_contains="License", |
There was a problem hiding this comment.
Maybe we should also verify that a license is actually outputted. Maybe just add MIT here? Or a just any license that is in the output.
|
I noticed that |
Sounds like a good idea. Should I open a new issue for this? |
|
Thanks for your work, I guess we can close this now as it inspired @baszalmstra enough to make the |
4 participants
Description
Added a column for license in pixi list, this is the output now:
Fixes #4715
How Has This Been Tested?
Building locally and then
pixi listChecklist:
schema/model.py.