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

Export Target type from ouroboros-network, to be more convenient to users #448

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

smelc
Copy link
Contributor

@smelc smelc commented Feb 8, 2024

Changelog

- description: |
    Export Target type from ouroboros-network, to be more convenient to users
# 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 #447

Fixes #449

How to trust this PR

User-facing API only

Checklist

  • Commit sequence broadly makes sense and commits have useful messages
  • Self-reviewed the diff

@smelc smelc force-pushed the smelc/expose-target-type-from-network branch from 86b93cd to 1bb2df1 Compare February 8, 2024 10:56
@smelc smelc marked this pull request as ready for review February 8, 2024 11:17
Copy link
Contributor

@carbolymer carbolymer left a comment

Choose a reason for hiding this comment

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

LGTM! I think it was @Jimbo4350's idea with reexposing stuff. I'm leaving approval to him if this is the direction we want to follow.

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.

We need to discuss how we want to expose things on a module basis. Can you create an issue for this? We have strayed far from what was originally envisioned with the Cardano.Api and Cardano.Api.Shelley but I am partly to blame :).

@smelc
Copy link
Contributor Author

smelc commented Feb 8, 2024

@Jimbo4350> issue created: #449

newhoggy pushed a commit that referenced this pull request Mar 11, 2024
…curences-5

Replace usages of assertFileOccurences (5/6)
Copy link

github-actions bot commented May 1, 2024

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions bot added the Stale label May 1, 2024
@Jimbo4350
Copy link
Contributor

I think this is fine. The alternative to this would be users traversing the ouroboros-network. Exposing this here would likely help inform how the ouroboros-network can be consumed.

@smelc smelc force-pushed the smelc/expose-target-type-from-network branch from 1bb2df1 to 39b0177 Compare June 13, 2024 13:18
@smelc smelc enabled auto-merge June 13, 2024 13:18
@smelc smelc disabled auto-merge June 13, 2024 13:46
@smelc smelc merged commit 3e0c234 into main Jun 13, 2024
21 of 32 checks passed
@smelc smelc deleted the smelc/expose-target-type-from-network branch June 13, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rethink how we expose our dependencies to users [BUG] - Re-export Target type for queryNodeLocalState
3 participants