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

Directly Run Credentials Process #1866

Closed
wants to merge 1 commit into from
Closed

Conversation

ijrsvt
Copy link

@ijrsvt ijrsvt commented Oct 2, 2022

@ijrsvt ijrsvt requested a review from a team as a code owner October 2, 2022 17:56
@skmcgrail
Copy link
Member

Sorry we can't accept this pull request due to several issues:

  • This would be a backwards incompatible change in behavior users who expect that the shell is being loaded for the sourcing of environment configuration. This would need minimally be gated behind some sort of configuration value to toggle the desired behavior. Likely something that can be toggled programmatically when calling config.LoadDefaultConfig
  • The given implementation lacks parsing/lexing of the command necessary to support commands that take arguments. Currently the way this is invoking the process it would pass the arguments as a single argument to the process, which would be incorrect.
  • Windows platforms would also need their own level of special handling of command argument parsing that are specific to how commands are pared there.

@skmcgrail
Copy link
Member

skmcgrail commented Oct 18, 2022

See this issue that was originally cut towards the Go repository #2879 which covers tracking this as a cross-sdk issue, as this is not just limited to the Go SDK.

@RanVaknin RanVaknin closed this Jan 26, 2023
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.

3 participants