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 certificates support in createCompatibleSignedTx. #691

Conversation

carbolymer
Copy link
Contributor

@carbolymer carbolymer commented Nov 25, 2024

Changelog

- description: |
    Add certificates support in `createCompatibleSignedTx`.
# uncomment types applicable to the change:
  type:
   - feature        # introduces a new feature
   - breaking       # the API has changed in a breaking way
  # - compatible     # the API has changed but is non-breaking
  # - optimisation   # measurable performance improvements
  # - refactoring    # QoL changes
  # - bugfix         # fixes a defect
  # - test           # fixes/modifies tests
  # - maintenance    # not directly related to the code
  # - release        # related to a new release preparation
  # - documentation  # change in code docs, haddocks...

Context

This PR provides certificates support in createCompatibleSignedTx.

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • New tests are added if needed and existing tests are updated. See Running tests for more details
  • Self-reviewed the diff

@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch from 2b524fd to 1700b02 Compare November 25, 2024 20:11
@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch 7 times, most recently from 5ba5b1f to df09cd9 Compare November 29, 2024 16:32
@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch 3 times, most recently from 1192ecc to 2bf3997 Compare January 3, 2025 16:42
@carbolymer carbolymer changed the title Add certs support in compatible Tx building Add certificates support in createCompatibleSignedTx. Jan 3, 2025
@carbolymer carbolymer marked this pull request as ready for review January 3, 2025 16:44
Copy link
Contributor

@smelc smelc 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 leaving the approval to @Jimbo4350 because I'm not super familiar with the underlying logic but LGTM from a high-level point of view.

cardano-api/internal/Cardano/Api/Tx/Compatible.hs Outdated Show resolved Hide resolved
cardano-api/internal/Cardano/Api/Tx/Compatible.hs Outdated Show resolved Hide resolved
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM! You can squash the commits.

@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch 2 times, most recently from 60194a8 to 7d89505 Compare January 13, 2025 21:07
@carbolymer carbolymer enabled auto-merge January 13, 2025 21:07
@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch from 7d89505 to 9079865 Compare January 13, 2025 21:18
@carbolymer carbolymer force-pushed the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch from 9079865 to fa8ccc1 Compare January 13, 2025 21:21
@carbolymer carbolymer added this pull request to the merge queue Jan 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Jan 13, 2025
@carbolymer carbolymer merged commit 2576829 into master Jan 14, 2025
70 checks passed
@carbolymer carbolymer deleted the mgalazyn/fix/add-certificates-support-to-compatible-tx-building branch January 14, 2025 08:23
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