-
Notifications
You must be signed in to change notification settings - Fork 433
Simplified pipelines and made deeper reuse across them #4160
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
Conversation
Removed the JavaSE ports screemshot tests since it was badly done. Will revisit this in a future PR.
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.
💡 Codex Review
CodenameOne/scripts/build-android-app.sh
Lines 195 to 199 in 735e41e
| ANDROID_TEST_SOURCE_DIR="$SCRIPT_DIR/device-runner-app/androidTest" | |
| ANDROID_TEST_ROOT="$GRADLE_PROJECT_DIR/app/src/androidTest" | |
| ANDROID_TEST_JAVA_DIR="$ANDROID_TEST_ROOT/java/${PACKAGE_PATH}" | |
| if [ -d "$ANDROID_TEST_ROOT" ]; then | |
| ba_log "Removing template Android instrumentation tests from $ANDROID_TEST_ROOT" |
In build-android-app.sh, the instrumentation test path is built with ${PACKAGE_PATH} but the earlier package-name definitions were removed. Because the script runs with set -u, reaching this line aborts the build before any tests are copied or Gradle runs, so the Android sample cannot be built.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
scripts/build-ios-app.sh
Outdated
| CN1_VERSION="8.0-SNAPSHOT" | ||
| WORK_DIR="scripts/hellocodenameone" | ||
| rm -rf "$WORK_DIR"; mkdir -p "$WORK_DIR" |
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.
Remove checked-in iOS app before build
build-ios-app.sh now wipes scripts/hellocodenameone (rm -rf "$WORK_DIR") but the skeleton generation step was removed, so the directory stays empty while the subsequent xmlstarlet edits expect pom.xml to exist. Running the script will therefore stop immediately with a missing-file error before any iOS project is built.
Useful? React with 👍 / 👎.
scripts/build-ios-app.sh
Outdated
| EXTRA_MVN_ARGS+=("-Dcodenameone.version=${CN1_VERSION}") | ||
|
|
||
| SETTINGS_FILE="$APP_DIR/common/codenameone_settings.properties" | ||
| if [ ! -f "$SETTINGS_FILE" ]; then | ||
| bia_log "codenameone_settings.properties not found at $SETTINGS_FILE" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| set_property() { | ||
| local key="$1" value="$2" | ||
| if grep -q "^${key}=" "$SETTINGS_FILE"; then | ||
| if sed --version >/dev/null 2>&1; then | ||
| sed -i -E "s|^${key}=.*$|${key}=${value}|" "$SETTINGS_FILE" | ||
| else | ||
| sed -i '' -E "s|^${key}=.*$|${key}=${value}|" "$SETTINGS_FILE" | ||
| fi | ||
| else | ||
| printf '\n%s=%s\n' "$key" "$value" >> "$SETTINGS_FILE" | ||
| fi | ||
| } | ||
|
|
||
| set_property "codename1.packageName" "$PACKAGE_NAME" | ||
| set_property "codename1.mainName" "$MAIN_NAME" | ||
|
|
||
| # Ensure trailing newline | ||
| tail -c1 "$SETTINGS_FILE" | read -r _ || echo >> "$SETTINGS_FILE" |
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.
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Android screenshot updatesCompared 4 screenshots: 1 matched, 1 updated, 2 missing references.
Native Android coverage
|









Removed the JavaSE ports screemshot tests since it was badly done. Will revisit this in a future PR.