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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify command execution to check for local context and use default value if defined 馃Ш #4047

Closed
8 tasks done
Adam-it opened this issue Nov 11, 2022 · 5 comments
Closed
8 tasks done
Assignees
Milestone

Comments

@Adam-it
Copy link
Contributor

Adam-it commented Nov 11, 2022

馃幆 Aim

The aim of this issue is to modify CLI command execution path to first check if there is any context file in the current path the user is in and if so check if any of the options defined in it may be used in order to populate the currently executed command. Full description with examples is added to the related issue.

Related issue

#3896

Spec

  • checks the current working directory for a context file
  • if context file found and parsing failed, throws an error and fails command execution
  • if no context file found or context file found but doesn't have any context values, executes command using arguments provided by the user
  • if context file found and parsed, reads values from options specified in the context
  • using information about the current command, builds an option object that contains only options defined in the command
  • merges the options object from the context with options specified by the user, where the options specified by the user override values from the context
  • validates provided options using command's validators and reports any issues
  • logs debug information about retrieving context and applying to command so that in case of an error we have sufficient information to debug what's causing the error
@Adam-it Adam-it self-assigned this Nov 11, 2022
@Adam-it Adam-it changed the title modify command execution to check for local context and use default value if defined 馃Ш Modify command execution to check for local context and use default value if defined 馃Ш Nov 11, 2022
@Adam-it Adam-it added enhancement needs peer review Needs second pair of eyes to review the spec or PR labels Nov 11, 2022
@Adam-it
Copy link
Contributor Author

Adam-it commented Nov 11, 2022

@pnp/cli-for-microsoft-365-maintainers any comments before I start 馃槈?

@waldekmastykarz
Copy link
Member

Let's bring the relevant pieces of the epic to this issue, so that we can easily validate the PR without having to dissect the complete epic for what's relevant. We should define at minimum:

  • where we load context from
  • how we handle no context or errors while loading context
  • how we handle the case when context defines options that are not a part of the command (be sure to spec the difference for commands that do and don't support unknown options, this latter could be problematic!)
  • how we handle the case when user defines a value for an option when it's also set in context

@waldekmastykarz waldekmastykarz removed the needs peer review Needs second pair of eyes to review the spec or PR label Nov 13, 2022
@waldekmastykarz
Copy link
Member

@Adam-it do you want to take a stab at this spec or shall I do it?

@Adam-it
Copy link
Contributor Author

Adam-it commented Dec 10, 2022

@Adam-it do you want to take a stab at this spec or shall I do it?

If you have time please do 馃槏.
I will not be able to start this before Christmas 馃槬.

@Adam-it
Copy link
Contributor Author

Adam-it commented Feb 5, 2023

I should find time to focus on this one 馃憤

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

Successfully merging a pull request may close this issue.

4 participants