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

feat: add support for saving public key to file using -output flag #318

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

pmpbaptista
Copy link
Contributor

Description

  • Introduced the -output flag to allow users to specify a file path for saving the public key.
  • Updated the printPublicKey function to write the public key to the specified file, or to stdout if no file is provided.
  • Modified the create and migrate cases to support the -output flag when printing the public key.
  • Ensured backward compatibility by retaining stdout output when -output is not used.

Fixes # (issue)

Type of change

Please select all options that apply to this change:

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update

Checklist:

Confirm you have completed the following steps:

  • My code follows the style of this project.
  • I have performed a self-review of my code.
  • I have made corresponding updates to the documentation.
  • I have added/updated unit tests to cover my changes.

@@ -90,7 +104,9 @@ func main() {
config.RegisterCommandLineFlags()
flag.Usage = cliUsage
flag.BoolVar(&overwrite, "f", false, "Overwrite existing key if it exists")
flag.StringVar(&outputFile, "output", "", "Path to save the public key")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The output of tesla-keygen --help becomes:

  -keyring-type type
    	Keyring type (file|keychain|pass). Defaults to $TESLA_KEYRING_TYPE.
  -output string
    	Path to save the public key
  -session-cache file
    	Load session info cache from file. Defaults to $TESLA_CACHE_FILE.

To make the argument of -outupt "file" rather than "string", which is more consistent with the other options, please change the help string to:

Save public key to `file`. Defaults to stdout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestion applied


// If outputFile is provided, write to file, else write to stdout
var out io.Writer = os.Stdout
if outputFile != "" {
Copy link
Collaborator

@sethterashima sethterashima Oct 3, 2024

Choose a reason for hiding this comment

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

Currently usageText says:

The program writes the public key to stdout (except when deleting a key)

Please change to

The program writes the public key to stdout (except when deleting a key or setting the output location with -output)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestion applied

- Introduced the `-output` flag to allow users to specify a file path for saving the public key.
- Updated the `printPublicKey` function to write the public key to the specified file, or to stdout if no file is provided.
- Modified the `create` and `migrate` cases to support the `-output` flag when printing the public key.
- Ensured backward compatibility by retaining stdout output when `-output` is not used.
Copy link
Collaborator

@sethterashima sethterashima left a comment

Choose a reason for hiding this comment

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

Thanks!

@sethterashima sethterashima merged commit d472dfc into teslamotors:main Oct 4, 2024
1 check passed
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.

2 participants