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

Update cardano-api to 8.45.0.0 #720

Merged
merged 6 commits into from
Apr 16, 2024
Merged

Conversation

smelc
Copy link
Contributor

@smelc smelc commented Apr 15, 2024

Changelog

- description: |
    Update cardano-api to 8.45.0.0
# 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
  # - improvement    # QoL changes e.g. refactoring
  # - 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

Fixes #705

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

@smelc
Copy link
Contributor Author

smelc commented Apr 16, 2024

This previous run shows that the new step cabal test all does indeed catch golden files errors: https://github.com/IntersectMBO/cardano-cli/actions/runs/8702482757/job/23866620767?pr=720 (this pipeline ran on a state where the golden metadata file hadn't been adapted).

@smelc smelc force-pushed the smelc/update-cardano-api-to-8.45.0.0 branch from 45aab0d to 68a627a Compare April 16, 2024 09:41
@smelc smelc force-pushed the smelc/update-cardano-api-to-8.45.0.0 branch from 68a627a to 0f0a8f1 Compare April 16, 2024 09:42
@smelc smelc marked this pull request as ready for review April 16, 2024 09:42

-- | Test that converting a CC extended verification key yields the expected result.
-- | Execute me with:
-- @cabal test cardano-cli-golden --test-options '-p "/golden extended cc vkey to non extended vkey/"'@
Copy link
Contributor

@newhoggy newhoggy Apr 16, 2024

Choose a reason for hiding this comment

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

It's not ideal to have these comments here as they can get out of sync with the function name.

Maybe it's worth modifying tast-discover to include the option of reproducing the function name in the test name if that's what we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally like those comments, because they allow me to run one test without having to think about the complex shell command to enter. So far people received those new comments favorably, you're the first one to remark them 🙂 So I'm tempted to leave it here 😇

# This step is temporary. We'll soon remove it and check instead
# that golden files have not been modified when creating them below.
- name: Test all
run: cabal test all --enable-tests --test-show-details=direct -j1
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused by the role this step has. Can you elaborate what problem you are solving with this temporary workaround?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is more context here on Slack. Gist is that we were not checking that golden files are correct.

@newhoggy newhoggy self-requested a review April 16, 2024 14:06
@smelc smelc added this pull request to the merge queue Apr 16, 2024
Merged via the queue into main with commit 0048f11 Apr 16, 2024
17 checks passed
@smelc smelc deleted the smelc/update-cardano-api-to-8.45.0.0 branch April 16, 2024 14:56
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.

key non-extended-key doesn't support extended CC keys
2 participants