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

Allow and add warning for HTTP anchor-data #979

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

palas
Copy link
Contributor

@palas palas commented Nov 26, 2024

Changelog

- description: |
    Modified anchor-data checking to allow HTTP schema for testing purposes
  type:
  - feature

Context

Having only support for HTTPS and IPFS is cumbersome, because it would require obtaining an SSL/TLS certificate and a domain for running the tests, or publishing to IPFS with every test (whenever the anchor-data changes).

This PR allows HTTP schema but shows a warning when it is used.

It also ensures we use "scheme" throughout and not "schema" (for self-consistency and consistency with IANA nomenclature), and adds documentations for some functions.

How to trust this PR

There are plenty of tests so, in my opinion, it is mainly about documentation and code structure. I tested the warning manually, let me know if we should add an automated test for it.

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

@palas palas added the enhancement New feature or request label Nov 26, 2024
@palas palas self-assigned this Nov 26, 2024
@palas palas requested a review from mkoura November 26, 2024 11:40
@palas palas force-pushed the allow-http-anchor-data-but-warn branch from 01e0655 to 0b06c79 Compare November 26, 2024 13:11
@palas palas requested a review from Jimbo4350 November 26, 2024 13:26
cardano-cli/src/Cardano/CLI/Run/Hash.hs Outdated Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/Run/Hash.hs Outdated Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/Run/Hash.hs 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 but please remove the errors dependency. You can use hoistMaybe.

@palas palas force-pushed the allow-http-anchor-data-but-warn branch from dd3518e to 8251f1f Compare November 27, 2024 12:28
@palas palas enabled auto-merge November 27, 2024 12:33
@palas palas added this pull request to the merge queue Nov 27, 2024
Merged via the queue into master with commit 8216024 Nov 27, 2024
27 checks passed
@palas palas deleted the allow-http-anchor-data-but-warn branch November 27, 2024 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants