Skip to content

Use same API for signingCb in all places that can be crypto signed#1621

Merged
implausible merged 2 commits into
nodegit:masterfrom
implausible:fix/unify-signing-callback-apis
Feb 5, 2019
Merged

Use same API for signingCb in all places that can be crypto signed#1621
implausible merged 2 commits into
nodegit:masterfrom
implausible:fix/unify-signing-callback-apis

Conversation

@implausible

@implausible implausible commented Feb 5, 2019

Copy link
Copy Markdown
Member

Any place where we implement a callback pattern for signing commits/tags follows the same API:

type SigningCB = (content: string) => { code: number, field?: string, signedData?: string };

What still needs to be done:

  • Add new test cases for amend
  • Add new test cases for commit
  • Add new test cases for rebase
  • Add new test cases for tag

Any place where we implement a callback pattern for signing commits/tags follows the same API:

`type SigningCB = (content: string) => { code: number, field?: string, signedData?: string };`
@implausible implausible force-pushed the fix/unify-signing-callback-apis branch from 79a87dd to 874e653 Compare February 5, 2019 17:30
@implausible implausible changed the title [WIP] Use same API for signingCb in all places that can be crypto signed Use same API for signingCb in all places that can be crypto signed Feb 5, 2019
Comment thread test/tests/commit.js
Comment thread test/tests/commit.js
Comment thread test/tests/rebase.js Outdated
@dabutvin

dabutvin commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

FWIW I just verified the changes here in commit.js still lead to a verified commit signature in the github ui using

repo.createCommitWithSignature(
    'HEAD',
    author,
    committer,
    commitMessage,
    oid,
    [parent],
    'gpgsig',
    onSignature
  )

...

function onSignature(tosign) { ... }

image

@implausible

Copy link
Copy Markdown
Member Author

@dabutvin Sorry to shift this around from underneath ya 😄. We are hoping to stabilize by release of v0.25.0.

@implausible implausible merged commit 6183587 into nodegit:master Feb 5, 2019
@implausible implausible deleted the fix/unify-signing-callback-apis branch February 5, 2019 20:43
@dabutvin

dabutvin commented Feb 5, 2019

Copy link
Copy Markdown
Contributor

no worries, the update to take the head parameter is a huge improvement for using this.

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