-
Notifications
You must be signed in to change notification settings - Fork 3
chore: bdk-ffi submodule pre-0.30.0 #11
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
base: main
Are you sure you want to change the base?
Conversation
Revert "ci: pin dart sdk to 3.9.2 for stable formatting" This reverts commit 4deb4f5. chore: ci version
|
@Johnosezele im trying to fix ci failures with this pr, but would love for you to take a look and make sure things look right and we need all the changes made here and there aren't changes that shouldn't be in here ultimately would like ci fixed first before other prs are merged, and if you want to do a pr to fix ci if this one doesnt look right then im all for that |
| - name: Format check | ||
| run: dart format --output=none --set-exit-if-changed . | ||
| run: dart format --output=none --set-exit-if-changed lib test examples | ||
|
|
||
| - name: Analyze | ||
| run: dart analyze --fatal-infos --fatal-warnings | ||
| run: dart analyze --fatal-infos --fatal-warnings lib test examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently the workflow formats/analyzes only lib, test & examples, if we want ci to guard the demo we should add bdk_demo/lib and maybe bdk_demo/tests
| uses: dart-lang/setup-dart@v1 | ||
| with: | ||
| sdk: "3.9.2" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since our bdk_demo is a flutter app we'd have to install flutter in the job
- name: Setup Flutter
uses: subosito/flutter-action@v2
with:
channel: stable
|
|
||
| - name: Pub get | ||
| run: dart pub get | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so then we add this before formating/analzing bdk_demo/
- name: Pub get (bdk_demo)
working-directory: bdk_demo
run: flutter pub get
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
after the flutter pub get stepp, we can also do flutter analyze for bdk_demo/
- name: Analyze (bdk_demo)
working-directory: bdk_demo
run: flutter analyze
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe in the future when we add widget/integration test to demo we can follow up with
- name: Test (bdk_demo)
working-directory: bdk_demo
run: flutter test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to regenerate our bindings on this update, it was successful, I can see some new types were added
|
@Johnosezele thanks for reviewing! I couldn't tell if each comment was a suggestion item or necessary action item for this PR, but I tried to add a commit based on what I thought you might be going for in terms of action items 9b53534 and tests pass. but if what I did was wrong or not necessary let me know, and also let me know specifically what you would like added/changed/removed and I can make it happen. |
No description provided.