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

Improve APIs and types (BREAKING) #16

Merged
merged 17 commits into from
Jul 24, 2023
Merged

Conversation

olistic
Copy link
Contributor

@olistic olistic commented Jun 30, 2023

Summary

This PR contains breaking changes. Review commit-by-commit recommended.

This PR intends to improve general quality of the package by bringing consistency to API method naming (breaking change), type naming (breaking change) and removing presumably deprecated code (breaking change).

Additionally, it improves the structure of the project.

Depends on #14 and #15.

index.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This commit was reverted in aef8337 as I realized this function is not broadcasting any transaction (as I originally thought) but actually sending BTC to a recipient.

@olistic

This comment was marked as outdated.

@victorkirov
Copy link
Member

For some reason, GitHub is not updating this PR to show the latest commit in my branch: develop...olistic:sats-connect:types.

It's been happening for me on GitHub recently as well. Most reliable way I've found of fixing it is editing the PR base branch, setting it to the same one. When it fires off the save, it seems to update the upstream commits.

@olistic
Copy link
Contributor Author

olistic commented Jul 15, 2023

@victorkirov @yknl Rebased.

@olistic
Copy link
Contributor Author

olistic commented Jul 20, 2023

@victorkirov @yknl Rebased.

Any initial feedback on this PR? Anything I can do to make reviewing easier?

Copy link
Member

@victorkirov victorkirov left a comment

Choose a reason for hiding this comment

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

Sorry for the delay. This looks good to me 👍 The restructure and naming are definitely overall improvements.
Will await feedback from @yknl before the final merge

Copy link
Contributor

@yknl yknl left a comment

Choose a reason for hiding this comment

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

LGTM thanks @olistic

@yknl yknl merged commit 9f0d3f4 into secretkeylabs:develop Jul 24, 2023
2 of 3 checks passed
@olistic olistic deleted the types branch July 25, 2023 14:25
m-aboelenein added a commit that referenced this pull request Aug 29, 2023
* added signMessage method to provider

* throw error on missing options

* Include send btc transaction function

* Remove console.log

* convert to amountSats

* make message optional

* Allow to pass a custom Bitcoin provider (#14)

* Allow to pass a custom Bitcoin provider

* Extract GetBitcoinProviderFunc type

* Add senderAddress and allow multiple recipients (#10)

* Add senderAddress and allow multiple recipients

* Use Recipient in payload

* Use bigint type for amountSats

* Improve readability and error messages (#15)

* Improve readability

* Clean up error messages

* Remove duplicate line

* Improve error message

---------

Co-authored-by: Victor Kirov <victor.kirov@gmail.com>

* Add NPM pull request preview publish github workflow (#22)

* Add NPM pull request preview publish github workflow

* Add NPM pull request preview publish github workflow

* fix build script

* add npmrc file

* add npm token

* add npmrc file

* change package repository to npm

* update release workflow

* release v0.5.0 (#23)

* Improve APIs and types (BREAKING) (#16)

* Remove unused type

* Rename AddressPurposes enum for best practices

* Extract call types into separate file

* Remove unused CallMethod enum

* Extract provider types into separate file

* Rename directories for consistency

* Rename sendBtcTransaction -> sendTransaction

* Extract transactions types to separate file

* Extract networks module

* Improve types

* Use [] instead of Array<> for consistency

* Fix copy-paste oversight

* Move index.ts inside src

* Use import type for types

* Revert sendBtcTransaction -> sendTransaction renaming

* Simplify types

---------

Co-authored-by: Victor Kirov <victor.kirov@gmail.com>

* Fix bigInt serialisation Issue (#25)

* serialize recipient array

* Fix AddressPurposes type

* Revert AddressPurpose name change

* Rename const

* Check recipient type

* chore: npm publish with prerelease tag if prerelease option selected (#27)

* chore: update release-package.yml syntax (#28)

* Release v1.0.0 (#24)

release v1.0.0

Co-authored-by: Tim Man <tim@secretkeylabs.com>

---------

Co-authored-by: Ken Liao <yukanliao@gmail.com>
Co-authored-by: Imamah-Zafar <imamah.zafar@tintash.com>
Co-authored-by: Duska.T <55587184+DuskaT021@users.noreply.github.com>
Co-authored-by: Matías Olivera <moliverafreire@gmail.com>
Co-authored-by: Imamah-Zafar <88320460+Imamah-Zafar@users.noreply.github.com>
Co-authored-by: Victor Kirov <victor.kirov@gmail.com>
Co-authored-by: Tim Man <tim@secretkeylabs.com>
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.

None yet

3 participants