Skip to content

Conversation

@ViktorT-11
Copy link
Contributor

@ViktorT-11 ViktorT-11 commented Jan 16, 2025

This PR bumps the lightning-terminal dependency for the wasm-client + mobile to v0.14.0-alpha. It also bumps the go version of the lightning-node-connect package to 1.22.6.

We also bump the upload-artifact action to v4

@ViktorT-11 ViktorT-11 requested a review from jamaljsr January 16, 2025 20:19
@ViktorT-11 ViktorT-11 force-pushed the litd-v0_14_0 branch 2 times, most recently from a74cbc4 to fee1bac Compare January 16, 2025 21:36
Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

tACK on the updated WASM binary. I tested this in TW.

In /mobile we replace faraday to force it to be set to v0.2.13-alpha, as
we'll else get an ambiguous import during `make mobile` of which
faraday/frdrpc to use. This forces the dependency to the faraday/frdrpc
of the faraday v0.2.13-alpha release, where as the litcli dependency
uses the first actual separate tagged release for frdrpc v0.1.0
Comment on lines +212 to +216
// Force faraday to use the master version as we'll either get an ambigious import if we try use faraday v0.2.13-alpha
// as an indirect import here instead of indirectly importing faraday/frdrpc v1.0.0, or get an unkown revision for the
// frdrpc if we don't include this replace. Once faraday v0.2.14-alpha is released, this should be resolved and this
// replace can be removed in favour of importing that.
replace github.com/lightninglabs/faraday => github.com/lightninglabs/faraday v0.2.13-alpha.0.20241118202659-a3aba5b7ea49
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to include this replace which isn't ideal, especially since it's referencing the unreleased master version, but couldn't figure any other way of getting the dependencies to work.

@ViktorT-11 ViktorT-11 requested a review from jamaljsr January 17, 2025 19:47
Copy link
Member

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@ellemouton ellemouton self-requested a review January 19, 2025 16:49
Copy link
Member

@ellemouton ellemouton left a comment

Choose a reason for hiding this comment

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

lgtm!

github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a // indirect
github.com/jackc/pgtype v1.14.0 // indirect
github.com/jackc/pgx/v4 v4.18.2 // indirect
github.com/jackc/puddle v1.3.0 // indirect
Copy link
Member

Choose a reason for hiding this comment

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

can squash this commit into previous one which does the go.mod update :) but non-blocking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason i separated these into different commits is that the changes between those 2 commits are actually unrelated from each other, as project hadn't run go mod tidy for the previously merged PR. So even though they're both changing the go.mod file, neither of the commits are actually related :).

I should have clarified this, and In hindsight maybe it'd make sense to include the go mod tidy commit as the first commit in the PR for clarification. But choosing to merge this as is to avoid another CI run.

@ViktorT-11
Copy link
Contributor Author

Thanks a lot for the reviews @ellemouton & @jamaljsr 🎉🙏!!

@ViktorT-11 ViktorT-11 merged commit 5267d1d into lightninglabs:master Jan 20, 2025
5 checks passed
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.

3 participants