Skip to content

Optimize the workflow for dart CI#988

Merged
spacebear21 merged 1 commit intopayjoin:masterfrom
benalleng:dart-workflow-optimize
Aug 21, 2025
Merged

Optimize the workflow for dart CI#988
spacebear21 merged 1 commit intopayjoin:masterfrom
benalleng:dart-workflow-optimize

Conversation

@benalleng
Copy link
Copy Markdown
Collaborator

@benalleng benalleng commented Aug 21, 2025

This is a simplification for the dart build and test workflow for linux and macos

This is the part 1 to split up #980 into more manageable chunks

Pull Request Checklist

Please confirm the following before requesting review:

  • A human has reviewed every single line of this code before opening the PR (no auto-generated, unreviewed LLM/robot submissions).
  • I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Aug 21, 2025

Pull Request Test Coverage Report for Build 17137104182

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 86.156%

Totals Coverage Status
Change from base Build 17134043038: 0.0%
Covered Lines: 7885
Relevant Lines: 9152

💛 - Coveralls

@benalleng benalleng force-pushed the dart-workflow-optimize branch 2 times, most recently from 15a2e31 to a6b923e Compare August 21, 2025 18:34
@benalleng benalleng requested a review from spacebear21 August 21, 2025 18:47
Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

Concept ACK, I have some minor suggestions

Comment thread .github/workflows/dart.yml Outdated
strategy:
matrix:
os: [ubuntu-latest, macos-13]
os: [ubuntu-latest, macos-15]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we could use macos-latest?

Comment on lines +27 to +28
cargo build --features _test-utils --profile release
cargo run --features _test-utils --profile release --bin uniffi-bindgen -- --library target/release/$LIBNAME --language dart --out-dir dart/lib/
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This step appears to be the same regardless of the platform, could likely be moved before the if?

chmod +x ./scripts/generate_linux.sh
chmod +x ./scripts/generate_macos.sh
OS=$(uname -s)
ARCH=$(uname -m)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like ARCH is no longer needed

Comment on lines +15 to +18
if [[ "$OS" == "Darwin" ]]; then
LIBNAME=$MAC_LIBNAME
elif [[ "$OS" == "Linux" ]]; then
LIBNAME=$LINUX_LIBNAME
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if [[ "$OS" == "Darwin" ]]; then
LIBNAME=$MAC_LIBNAME
elif [[ "$OS" == "Linux" ]]; then
LIBNAME=$LINUX_LIBNAME
if [[ "$OS" == "Darwin" ]]; then
LIBNAME=libpayjoin_ffi.dylib
elif [[ "$OS" == "Linux" ]]; then
LIBNAME=libpayjoin_ffi.so

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: I'd name this generate_bindings.sh or something more plain english

@benalleng benalleng force-pushed the dart-workflow-optimize branch from a6b923e to a1ee3a8 Compare August 21, 2025 19:19
Comment thread .github/workflows/dart.yml Outdated
# Skip integration test on macOS due to Docker issues
run: |
if [ "${{ matrix.os }}" = "macos-13" ]; then
if [ "${{ matrix.os }}" = "macos-15" ]; then
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if [ "${{ matrix.os }}" = "macos-15" ]; then
if [ "${{ matrix.os }}" = "macos-latest" ]; then

Copy link
Copy Markdown
Collaborator Author

@benalleng benalleng Aug 21, 2025

Choose a reason for hiding this comment

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

i think that macos-latest on matrix.os returns a specific version in this case macos-14 until the end of the month when it will transition to macos-15. not 100% on this though

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

nvm you're right

@benalleng benalleng force-pushed the dart-workflow-optimize branch from a1ee3a8 to 58836ae Compare August 21, 2025 19:26
This is a simplification for the dart build and test workflow for linux
and macos
@benalleng benalleng force-pushed the dart-workflow-optimize branch from 58836ae to ab7fa2a Compare August 21, 2025 19:34
Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

utACK

@spacebear21 spacebear21 merged commit 53277da into payjoin:master Aug 21, 2025
17 of 18 checks passed
@benalleng benalleng deleted the dart-workflow-optimize branch March 27, 2026 15:38
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