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

Use list-like flags for flags that take a list of options #905

Closed

Conversation

josegonzalez
Copy link

@josegonzalez josegonzalez commented May 26, 2023

This PR migrates the existing flags that use a list of values for a single flag (--flag value1 value2) to a more common format of list of flags (--flag value1 --flag value2). This allows folks to more seamlessly switch from other CLI tools like docker or pack.

  • Tests are added/updated if needed
  • Docs are updated if needed

@coffee-cup coffee-cup added the release/major Author major release label May 26, 2023
@coffee-cup
Copy link
Contributor

coffee-cup commented May 26, 2023

Going to hold off on merging this PR for now since it requires a major version update as I am guessing the old way of specifying multiple args does not work anymore?

@josegonzalez
Copy link
Author

Yeah it doesn't. I was thinking that maybe we could introduce parsing of each option to see if there should be splitting but idk if there is a shell word-splitting library in rust we can pull in.

@coffee-cup
Copy link
Contributor

Honestly, I believe we originally had that form of list arg parsing initially and migrated it to what we have currently in this PR #679. I will discuss with the team the best option moving forward. Unfortunately, we are unlikely to make a major version update just for this change.

@josegonzalez
Copy link
Author

If we did shell-parsing of the values when there is only one value, would that be enough to get this into a minor?

@josegonzalez
Copy link
Author

One thing I noticed is that at least the pkgs flag seems to act as I desire with the latest stable release. Let me do a bit more testing to see if this PR is necessary (and when it is necessary, if at all).

@github-actions
Copy link
Contributor

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Jun 10, 2023
@josegonzalez
Copy link
Author

@coffee-cup is there anything I can do to help make this change more palatable?

@github-actions github-actions bot removed the stale The pull request is stale label Jun 11, 2023
@github-actions
Copy link
Contributor

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Jun 22, 2023
@github-actions github-actions bot removed the stale The pull request is stale label Jun 23, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2023

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Jul 3, 2023
@github-actions github-actions bot removed the stale The pull request is stale label Jul 4, 2023
@github-actions
Copy link
Contributor

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Jul 14, 2023
@github-actions github-actions bot removed the stale The pull request is stale label Jul 15, 2023
@github-actions
Copy link
Contributor

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Jul 26, 2023
@github-actions github-actions bot removed the stale The pull request is stale label Jul 30, 2023
@github-actions
Copy link
Contributor

This pull request is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The pull request is stale label Aug 10, 2023
This changes the parser from:

    --label one two three

to using the following format:

    --label one --label two --label three

This follows how the docker daemon exposes the flags.

Refs railwayapp#651
@josegonzalez
Copy link
Author

Is there any interest in this PR at all? If not, I can just close the PR.

@github-actions github-actions bot removed the stale The pull request is stale label Aug 16, 2023
@josegonzalez
Copy link
Author

Closing as there hasn't been any interest from upstream in reviewing/merging the PR.

@josegonzalez josegonzalez deleted the 651-list-like-labels branch August 20, 2023 03:23
@coffee-cup
Copy link
Contributor

Hi @josegonzalez,
Not that there hasn't been any interest in merging this. It is just a major breaking change so was planning on waiting to merge it with some other breaking changes in the future (no release date). I will revisit this in the future when those changes are made.

@josegonzalez
Copy link
Author

Just updating, Not sure what happened but seems like list-like flags (for labels at least) suddenly work in main (the latest nixpacks cli does locally anyways)! Maybe someone jingled some keys :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/major Author major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants