Skip to content

Conversation

Defelo
Copy link
Member

@Defelo Defelo commented Sep 24, 2025

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added/updated tests to cover my changes

@Defelo Defelo requested a review from a team as a code owner September 24, 2025 09:00
@google-cla
Copy link

google-cla bot commented Sep 24, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Defelo Defelo force-pushed the push-rwqvpyuskwqx branch 2 times, most recently from 57bb6fa to 9688332 Compare September 25, 2025 12:49
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

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

I'm not pretty sure if the output format is stable, but the change looks good.

@julienvincent can you take another look?

@Defelo Defelo force-pushed the push-rwqvpyuskwqx branch 2 times, most recently from d2a3ff2 to 5da2ed5 Compare October 2, 2025 04:34
@Defelo
Copy link
Member Author

Defelo commented Oct 2, 2025

Hey @yuja, thanks for the review! Regarding the output format stability, perhaps a better approach could be to use the ssh_key crate to parse the signature and extract the public key directly (see ssh_key::SshSig) instead of trying to parse the ssh-keygen output. Please let me know what you think.

@yuja
Copy link
Contributor

yuja commented Oct 2, 2025

Regarding the output format stability, perhaps a better approach could be to use the ssh_key crate to parse the signature and extract the public key directly (see ssh_key::SshSig) instead of trying to parse the ssh-keygen output. Please let me know what you think.

I don't have expertise, but I think it's better for us to rely on a single backend program or library.

@Defelo Defelo force-pushed the push-rwqvpyuskwqx branch from 5da2ed5 to d14e415 Compare October 8, 2025 10:44
@Defelo
Copy link
Member Author

Defelo commented Oct 8, 2025

@yuja Is there anything I can do to get this merged? I have already been invited to the contributors team, so according to the contributing guide I should be able to merge this myself now, however since you explicitly asked julienvincent to take another look in your approval two weeks ago, I thought I better ask first.

@yuja
Copy link
Contributor

yuja commented Oct 8, 2025

No. Feel free to merge the approved change if you addressed all review comments.

@Defelo Defelo added this pull request to the merge queue Oct 8, 2025
Merged via the queue into jj-vcs:main with commit b38dcbb Oct 8, 2025
30 checks passed
@Defelo Defelo deleted the push-rwqvpyuskwqx branch October 8, 2025 13:22
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