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.

Sign up for GitHub

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

Jump to bottom

Reduce exports from SotoSignerV4 #567

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Reduce exports from SotoSignerV4 #567

merged 1 commit into from
Jul 20, 2023

Conversation

adam-fowler
Copy link
Member

No description provided.

@adam-fowler adam-fowler requested a review from 0xTim July 19, 2023 13:13
@adam-fowler adam-fowler linked an issue Jul 19, 2023 that may be closed by this pull request
SotoSignerV4 exports all the symbols from NIOHTTP1 #530
Closed
@codecov
Copy link

codecov bot commented Jul 19, 2023 •
edited
Loading

Codecov Report

Merging #567 (fcc1b4c) into 7.x.x (15744ff) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##            7.x.x     #567   +/-   ##
=======================================
  Coverage   78.87%   78.87%           
=======================================
  Files          71       71           
  Lines        6292     6292           
=======================================
  Hits         4963     4963           
  Misses       1329     1329

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

0xTim
0xTim approved these changes Jul 20, 2023
View reviewed changes
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Reducing exports is always good - is there any reason why these are needed at all for v7?

@adam-fowler
Copy link
Member Author

Reducing exports is always good - is there any reason why these are needed at all for v7?

I think it is a good idea to export the symbols my public APIs use. Makes it easier for the end user.

@adam-fowler adam-fowler merged commit d838200 into 7.x.x Jul 20, 2023
@adam-fowler adam-fowler deleted the signer-imports branch July 20, 2023 08:23
@0xTim
Copy link
Member

0xTim commented Jul 20, 2023

Oh completely agree - are things like ByteBuffer going to be part of the public API though?

(This is a more general question as opposed to being specifically about this PR, I know removing all the future APIs is a multi step process)

@adam-fowler
Copy link
Member Author

ByteBuffer is part of the public API. I don't think that is going to change. Unless we get the bag of bytes type in the sodlib

@0xTim
Copy link
Member

0xTim commented Jul 20, 2023

🤦‍♂️ travel brain getting futures and byte buffers mixed up

adam-fowler added a commit that referenced this pull request Jul 24, 2023
@adam-fowler
Reduce exports from SotoSignerV4 (#567)
bf9ff5a
adam-fowler added a commit that referenced this pull request Jul 29, 2023
@adam-fowler
Reduce exports from SotoSignerV4 (#567)
386d560
adam-fowler added a commit that referenced this pull request Aug 12, 2023
@adam-fowler
Reduce exports from SotoSignerV4 (#567)
f02c943
adam-fowler added a commit that referenced this pull request Dec 24, 2023
@adam-fowler
Reduce exports from SotoSignerV4 (#567)
cc7bf70
adam-fowler added a commit that referenced this pull request Apr 5, 2024
@adam-fowler
Reduce exports from SotoSignerV4 (#567)
224a9e3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xTim 0xTim 0xTim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SotoSignerV4 exports all the symbols from NIOHTTP1
2 participants
@adam-fowler @0xTim