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

Commands and --instance option #31

Closed
mlocati opened this issue Sep 26, 2021 · 5 comments · Fixed by #35 · May be fixed by #38
Closed

Commands and --instance option #31

mlocati opened this issue Sep 26, 2021 · 5 comments · Fixed by #35 · May be fixed by #38
Labels
enhancement New feature or request

Comments

@mlocati
Copy link
Contributor

mlocati commented Sep 26, 2021

At the moment, every command accepts the --instance option.

BTW not all commands may implement the InstallationAwareInterface interface,

For example, here's the output of concrete help self-update:

Usage:
  self-update

Options:
  -h, --help               Display this help message
  -q, --quiet              Do not output any message
  -V, --version            Display this application version
      --ansi               Force ANSI output
      --no-ansi            Disable ANSI output
  -n, --no-interaction     Do not ask any interactive question
  -I, --instance=INSTANCE  Specify the concrete5 directory [default: "."]
  -v|vv|vvv, --verbose     Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug

Help:
  Update concrete.phar to the latest version.

IMHO we should add the --instance option only to commands implementing InstallationAwareInterface (not sure about how this can be done though).

@KorvinSzanto
Copy link
Member

Realistically I don't think we're going to have more than a single command that ignores this value. Does it really make sense to add extra code to every other command to avoid having a no-op option in that one command? I'm not so sure yet.

@mlocati
Copy link
Contributor Author

mlocati commented Sep 30, 2021

I'm thinking about a coding style checker/fixer that can use a concrete installation, but also may be disjointed from it (think for example at fixing the coding style of the code of a package)

@KorvinSzanto
Copy link
Member

That makes sense, though for that command you'd likely want a way to specify the package directory through an option the same way you can specify an instance, so I wonder if it makes sense to replace this --instance option with something more generic like --cwd or something so that it can handle both of those cases.

@mlocati
Copy link
Contributor Author

mlocati commented Sep 30, 2021

Yep, or --path, which is a little less technical

@KorvinSzanto
Copy link
Member

How about #38 to avoid --instance on self-update and we add a new issue to replace --instance with --path. That way we'll capture both changes in the changelog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants