Conversation
.github/workflows/ci.yml
Outdated
| distribution: 'temurin' | ||
| java-version: '17' | ||
| distribution: "temurin" | ||
| java-version: "17" |
There was a problem hiding this comment.
Yaml doesn't need any quoting right, drop entirely?
There was a problem hiding this comment.
this is just how vscode wants to format it by default, so it just makes my life easier when I edit the later and vscode automatically makes the same change - without me needing to special-case disabling save on format here.
There was a problem hiding this comment.
(well I suppose vscode won't add the quotes back if they are removed entirely, so that'd be fine too)
There was a problem hiding this comment.
But it also didn't add any quotations on any other string, like job names etc?
EDIT after concurrent post: yeah, the suggestion is to drop the quotes entirely.
| use android_activity::{ | ||
| AndroidApp, InputStatus, MainEvent, OnCreateState, PollEvent, | ||
| input::{InputEvent, KeyAction, KeyEvent, KeyMapChar, MotionAction}, | ||
| ndk, ndk_sys, | ||
| ndk, ndk_sys, AndroidApp, InputStatus, MainEvent, OnCreateState, PollEvent, | ||
| }; | ||
| use jni::{ |
There was a problem hiding this comment.
Might conflict with #24 where I ran cargo fmt -- --config imports_granularity=crate --config group_imports=StdExternalCrate.
There was a problem hiding this comment.
I guess it it came up with the same result then hopefully it's fine?
There was a problem hiding this comment.
but yeah the purpose of my opening this pr was to help you get a green light with your other prs, so fine with dropping this too
| cargo_ndk_args: [-t arm64-v8a -t armeabi-v7a -t x86_64 -t x86 -o app/src/main/jniLibs/] | ||
| cargo_ndk_args: | ||
| [ | ||
| -t arm64-v8a -t armeabi-v7a -t x86_64 -t x86 -o app/src/main/jniLibs/, | ||
| ] |
There was a problem hiding this comment.
Didn't notice the first time - can we just make this a multiline Yaml array if it formats it across multiple lines anyway?
There was a problem hiding this comment.
it does look ugly like this yeah
lets just land this though when if it passes CI, we can tidy this up some time and avoid another restart of the build
if this CI build fails then yeah can split it up
There was a problem hiding this comment.
(there's probably lots that could be improved about the ci.yaml workflow, and atm the aim was just intended to be a quick fix to get a greenlight and that just happend to pull in some automatic formatting changes when I added the -P 26 argument for the agdk-cpal build)
|
I wonder if CI should really be building for all of these architectures considering how slow it makes it |
2 participants
No description provided.