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
Copy link
Copy Markdown
Member

@implausible implausible commented Feb 5, 2019

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
Copy link
Copy Markdown
Contributor

dabutvin commented Feb 5, 2019

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
Copy link
Copy Markdown
Contributor

dabutvin commented Feb 5, 2019

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