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

feat: add new commands for pixi project {version|channel|platform|description} #579

Merged
merged 30 commits into from
Dec 20, 2023

Conversation

hadim
Copy link
Contributor

@hadim hadim commented Dec 18, 2023

A first proposal for #575

The idea is to add a few new pixi project commands:

  • pixi project description set
  • pixi project description get
  • pixi project channel list
  • pixi project channel remove
  • pixi project platform list
  • pixi project platform remove
  • pixi project platform add
  • pixi project version get
  • pixi project version set
  • pixi project version {major,minor,patch}: I did a first pass, but I feel the logic should live on rattler, no?

Tests are missing.

See also a few comments/questions below.

tests/project_tests.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Awesome work @hadim!

I left some comments with some requests for change. Please note the eprintln changes.

I would skip the version bump stuff in this pr and clean it up then we can merge it soon!

Wasn't expecting this to be fixed so soon!

src/cli/project/channel/list.rs Show resolved Hide resolved
src/cli/project/channel/list.rs Outdated Show resolved Hide resolved
src/cli/project/channel/remove.rs Outdated Show resolved Hide resolved
src/cli/project/channel/remove.rs Outdated Show resolved Hide resolved
src/cli/project/channel/remove.rs Outdated Show resolved Hide resolved
src/cli/project/platform/remove.rs Outdated Show resolved Hide resolved
src/cli/project/version/bump.rs Outdated Show resolved Hide resolved
src/cli/project/version/bump.rs Outdated Show resolved Hide resolved
src/cli/project/version/get.rs Show resolved Hide resolved
src/project/manifest.rs Show resolved Hide resolved
@hadim
Copy link
Contributor Author

hadim commented Dec 19, 2023

@ruben-arts LGTM.

Please let me know about the "CLI" tests in project_tests.rs (nothing there yet). I could not find a good way to capture stdout to check whether the CLI commands do what we want.

Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Almost looks good, one really small check.

src/environment.rs Show resolved Hide resolved
Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Thanks @hadim awesome addition!

@hadim
Copy link
Contributor Author

hadim commented Dec 20, 2023

@ruben-arts Thanks for the help and the review! All good on my side.

@ruben-arts ruben-arts merged commit 40200b6 into prefix-dev:main Dec 20, 2023
9 checks passed
@hadim hadim deleted the project_command branch December 20, 2023 16:10
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.

2 participants