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

add '--use-credential-process' to write command #443

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Conversation

cfbao
Copy link
Member

@cfbao cfbao commented Jan 15, 2024

Make it easier for people to start using BMX in credential_process, and reduce user errors.

https://d2l.slack.com/archives/G4P099831/p1705333949277619

@github-actions github-actions bot added language/csharp size/M A medium-sized PR. labels Jan 15, 2024
@cfbao cfbao force-pushed the use-credential-process branch from 6c631d6 to 28d3525 Compare January 16, 2024 14:27
@cfbao cfbao marked this pull request as ready for review January 16, 2024 14:29
@cfbao cfbao requested a review from a team as a code owner January 16, 2024 14:29
public const string Format = "Output format of AWS credentials";
public const string NonInteractive = "Run non-interactively without showing any prompts";
public const string CacheAwsCredentials = "Enables Cache for AWS tokens";
public const string CacheAwsCredentials =
"Enables Cache for AWS tokens. Implied if '--use-credential-process' is supplied";
Copy link
Member

Choose a reason for hiding this comment

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

It does make the bit about --cache a little confusing if the recommend way to start using bmx in credential process is to use -use-credential-process

Should we keep the flag but just not document it, suggest it should always be used indirectly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we can omit talking about it in the README, if that's what you mean.
But I think it's worth documenting all the flags in the CLI binary's help text for power users.
e.g. we use it here: https://github.com/Brightspace/vulcan-scratch/blob/0c8e6d00229339d9108240c03bb6602fdbba9705/slims/helper_scripts/lib/Invoke-AuthTokenRestMethod.psm1#L38

@cfbao cfbao merged commit 2c4493b into main Jan 17, 2024
9 checks passed
@cfbao cfbao deleted the use-credential-process branch January 17, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/csharp size/M A medium-sized PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants