Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions .github/workflows/pull-request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
contents: read
pull-requests: read
checks: write
id-token: write

Check failure on line 18 in .github/workflows/pull-request.yml

View workflow job for this annotation

GitHub Actions / Scan GitHub Actions workflows

excessive-permissions

pull-request.yml:18: overly broad permissions: id-token: write is overly broad at the workflow level

jobs:
# Code quality checks
Expand Down Expand Up @@ -247,6 +247,94 @@
name: "api-compat-reports"
path: build/api-compat/**

kotlin-migration-progress:
name: "Kotlin Migration Progress"
# Posts the Java-to-Kotlin conversion numbers on every pull request. The last step fails
# when a pull request adds Java to the published modules without the allow-new-java label.
if: github.event_name == 'pull_request'
timeout-minutes: 10
runs-on: ubuntu-latest
permissions:
contents: read
pull-requests: write
env:
COMMENT_IDENTIFIER: "<!-- kotlin-migration-progress -->"
steps:
# The default pull_request ref is the merge result; see the size-report job below for why
# that is the right side to measure and what happens on a conflicted pull request.
- name: "Checkout PR head"
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: "Checkout base branch"
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
ref: ${{ github.event.pull_request.base.sha }}
path: base
- name: "Test the progress script"
run: python3 -m unittest scripts/test_kotlin_migration_progress.py
- name: "Measure head and base"
id: measure
run: |
set -euo pipefail
read_metric() { grep "^$2=" "$1" | cut -d= -f2; }
python3 scripts/kotlin_migration_progress.py --format env > "${RUNNER_TEMP}/head.env"
python3 scripts/kotlin_migration_progress.py > "${RUNNER_TEMP}/head.md"
# The base is measured with the head's facade list, so editing the list cannot masquerade as progress.
python3 scripts/kotlin_migration_progress.py --root base --facades scripts/kotlin-migration-facades.txt --format env > "${RUNNER_TEMP}/base.env"
head_left=$(read_metric "${RUNNER_TEMP}/head.env" JAVA_LEFT_LOC)
base_left=$(read_metric "${RUNNER_TEMP}/base.env" JAVA_LEFT_LOC)
head_java=$(read_metric "${RUNNER_TEMP}/head.env" JAVA_LOC)
base_java=$(read_metric "${RUNNER_TEMP}/base.env" JAVA_LOC)
delta_left=$((head_left - base_left))
delta_java=$((head_java - base_java))
if [ "${delta_left}" -lt 0 ]; then
headline="This pull request converts **$((-delta_left)) lines** of Java to Kotlin."
elif [ "${delta_java}" -gt 0 ]; then
headline=":warning: This pull request **adds ${delta_java} lines** of Java to the published modules."
else
headline="No change to the Java left to convert."
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Headline hides warning when ratchet will fire

Low Severity

The headline branches on delta_left (non-facade Java) first, but the ratchet checks delta_java (all Java including facades). When delta_left < 0 and delta_java > 0 — e.g., a PR converts some non-facade Java while also growing a facade file — the headline cheerfully says "converts N lines" while the ratchet step fails the job. The PR comment gives no hint that the ratchet will fire, creating a confusing disconnect between the sticky comment and the red CI status.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6ade310. Configure here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Headline misreports when Java-left increases silently

Low Severity

The headline logic has a gap: when delta_left > 0 (Java left to convert increased) but delta_java <= 0 (total Java didn't grow), the else branch fires and reports "No change to the Java left to convert," which is factually wrong. The ratchet also doesn't catch this because it only checks delta_java > 0. A realistic trigger is extracting code from a facade file into a new non-facade internal class — total Java stays flat, but convertible Java rises. Neither the headline nor the ratchet flags the regression.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e31d469. Configure here.

{
echo "${COMMENT_IDENTIFIER}"
echo "### Kotlin migration progress"
echo
echo "${headline}"
echo
cat "${RUNNER_TEMP}/head.md"
} > "${RUNNER_TEMP}/comment.md"
cat "${RUNNER_TEMP}/comment.md" >> "${GITHUB_STEP_SUMMARY}"
echo "delta_java=${delta_java}" >> "${GITHUB_OUTPUT}"
# A pull_request from a fork gets a read-only token, so commenting would 403.
- name: "Find existing comment"
id: existing
if: github.event.pull_request.head.repo.full_name == github.repository
uses: peter-evans/find-comment@b30e6a3c0ed37e7c023ccd3f1db5c6c0b0c23aad # v4
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: "github-actions[bot]"
body-includes: ${{ env.COMMENT_IDENTIFIER }}
- name: "Create or update PR comment"
if: github.event.pull_request.head.repo.full_name == github.repository
uses: peter-evans/create-or-update-comment@e8674b075228eee787fea43ef493e45ece1004c9 # v5
with:
comment-id: ${{ steps.existing.outputs.comment-id }}
issue-number: ${{ github.event.pull_request.number }}
edit-mode: replace
body-path: ${{ runner.temp }}/comment.md
- name: "Ratchet: no new Java in the published modules"
if: >
steps.measure.outputs.delta_java > 0 &&
!contains(github.event.pull_request.labels.*.name, 'allow-new-java')
env:
DELTA_JAVA: ${{ steps.measure.outputs.delta_java }}
run: |
echo "This pull request adds ${DELTA_JAVA} lines of Java to android-core or android-kit-base." >&2
echo "New code in these modules is written in Kotlin. If the Java is unavoidable, add the" >&2
echo "allow-new-java label and explain why in the description." >&2
exit 1

automerge-dependabot:
name: "Save PR Number for Dependabot Automerge"
if: github.event_name == 'pull_request'
Expand Down
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ JDK 17 — `gradle.properties` sets `JAVA_VERSION` and every CI job installs Zul
intentional change and explain the diff in the PR. `scripts/check_api_dump.py --base origin/main`
tells you whether a changed class is a frozen contract (see `scripts/api-frozen-internals.txt`).
- Android lint — `./gradlew lint`; Kotlin lint — `./gradlew ktlintCheck`
- Migration progress — `scripts/kotlin_migration_progress.py` prints Java left to convert and the
Kotlin share of `android-core` and `android-kit-base` (CI job _Kotlin Migration Progress_).
- Binary compatibility with the last release — `scripts/api_compat_report.py` builds the release
AARs and runs japicmp against the latest version on Maven Central (CI job _Binary Compatibility_).
It compares what ships after R8, so it is the check that matters for consumers.
Expand Down
4 changes: 3 additions & 1 deletion docs/kotlin-migration/PLAYBOOK.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,6 @@ Whoever runs `Release – Draft` for unrelated work while migration pull request
- Titles follow the house convention: `refactor(core): convert internal.database tables to Kotlin`,
`build: …`, `ci: …`, `test: …`. A pure conversion has no `CHANGELOG.md` entry.
- A pull request that adds Java to `android-core/src/main` or `android-kit-base/src/main` needs a
stated reason; the direction of travel is Kotlin.
stated reason; the direction of travel is Kotlin. The **Kotlin Migration Progress** job posts the
conversion numbers on every pull request and fails when Java is added without the `allow-new-java`
label.
55 changes: 34 additions & 21 deletions docs/kotlin-migration/TRACKER.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ Java that converts: **16,550 LOC**. Java that stays by design (public facade, ki

No product code. Risk class L throughout.

| Done | PR | Title | Notes |
| ---- | --- | ----------------------------------------------------------------------------------- | ----------------------------------------------------------------------- |
| [ ] | 0.1 | `ci: report binary compatibility against the last published release` | `scripts/api_compat_report.py`; job **Binary Compatibility** |
| [ ] | 0.2 | `build: add public API dumps with binary-compatibility-validator` | `./gradlew apiCheck` in the Unit Tests job; `scripts/check_api_dump.py` |
| [ ] | 0.3 | `test: add Java and Kotlin consumer fixtures built against the published artifacts` | `settings-compat.gradle`, `compat/` |
| [ ] | 0.4 | `test: replace PowerMock with Mockito 5 in core and kit-base unit tests` | Prerequisite for converting classes the unit tests mock |
| [ ] | 0.5 | `test: expose explicit test seams for package-private core internals` | Prerequisite for stacks C and D |
| [ ] | 0.6 | `build(lint): build the custom lint jar from merged Java and Kotlin classes` | Prerequisite for converting `Logger` |
| [ ] | 0.7 | `docs: add the Kotlin migration playbook and tracker` | This document |
| Done | PR | Title | Notes |
| ---- | --- | ----------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------- |
| [ ] | 0.1 | `ci: report binary compatibility against the last published release` | `scripts/api_compat_report.py`; job **Binary Compatibility** |
| [ ] | 0.2 | `build: add public API dumps with binary-compatibility-validator` | `./gradlew apiCheck` in the Unit Tests job; `scripts/check_api_dump.py` |
| [ ] | 0.3 | `test: add Java and Kotlin consumer fixtures built against the published artifacts` | `settings-compat.gradle`, `compat/` |
| [ ] | 0.4 | `test: replace PowerMock with Mockito 5 in core and kit-base unit tests` | Prerequisite for converting classes the unit tests mock |
| [ ] | 0.5 | `test: expose explicit test seams for package-private core internals` | Prerequisite for stacks C and D |
| [ ] | 0.6 | `build(lint): build the custom lint jar from merged Java and Kotlin classes` | Prerequisite for converting `Logger` |
| [ ] | 0.7 | `docs: add the Kotlin migration playbook and tracker` | This document |
| [ ] | 0.8 | `ci: report Java to Kotlin migration progress on pull requests` | `scripts/kotlin_migration_progress.py`; job **Kotlin Migration Progress**; `allow-new-java` ratchet |

## Phase 1 · Build hygiene

Expand Down Expand Up @@ -132,18 +133,30 @@ zero frozen-class diff in `apiCheck` and an empty Binary Compatibility report.
| [ ] | 3.2 | `kits/KitManagerImpl` | 1,441 | H | Keep the Java declaration; move the body |
| [ ] | 3.3 | `internal/MParticleJSInterface` | 840 | H | Leave in Java |

## Checkpoints

QA points, not releases. Record the result of each in this section when it is reached.

| Point | After | Check |
| ----- | ----------------------------------- | ----------------------------------------------------------------------------------------------------- |
| M0 | Phase 0 and PR 1.1 | A deliberate removal of a public method on a scratch branch fails `apiCheck` and Binary Compatibility |
| M1 | Stacks A and D | Offline queue across process death, sessions, uploads |
| M2 | Stacks B, C and G | Identity flows, remote config, certificate pinning against production, push registration |
| M3 | Stacks E, F and H; Phase 1 complete | Full checklist; one third-party kit and the Rokt kit end to end on a minified consumer build |
| Gate | Before Phase 3 | Per-file go/no-go with the consumer fixture results |
| M4 | Phase 3 | Full checklist; the migration's release |
## Checkpoints and expected numbers

QA points, not releases. The **Kotlin Migration Progress** job reports "Java left to convert" and
"Kotlin share" on every pull request; the expected values below assume the stacks land in the
recommended order and that converted Java shrinks to roughly 65–80% of its length as Kotlin.
"Conversion progress" is exact: it is the share of the 16,550 baseline Java LOC that has been
converted.

| Point | After | Java left (LOC) | Conversion progress | Kotlin share (est.) | Check |
| ----- | ----------------------------------- | --------------: | ------------------: | ------------------: | ----------------------------------------------------------------------------------------------------- |
| M0 | Phase 0 and PR 1.1 | 16,550 | 0% | 12% | A deliberate removal of a public method on a scratch branch fails `apiCheck` and Binary Compatibility |
| | Stack A | 14,062 | 15% | 18–19% | |
| M1 | Stacks A and D | 11,336 | 32% | 24–26% | Offline queue across process death, sessions, uploads |
| | Stack B | 10,066 | 39% | 27–29% | |
| | Stack C | 9,159 | 45% | 30–32% | |
| M2 | Stacks B, C and G | 8,143 | 51% | 32–35% | Identity flows, remote config, certificate pinning against production, push registration |
| | Stack E | 7,141 | 57% | 35–38% | |
| | Stack F | 5,922 | 64% | 38–41% | |
| M3 | Stacks E, F and H; Phase 1 complete | 3,350 | 80% | 46–50% | Full checklist; one third-party kit and the Rokt kit end to end on a minified consumer build |
| Gate | Before Phase 3 | 3,350 | 80% | 46–50% | Per-file go/no-go with the consumer fixture results |
| M4 | Phase 3 | 0 | 100% | 56–60% | Full checklist; the migration's release |

The Kotlin share stops well short of 100% by design: 11,287 LOC of Java facade stay Java in 6.x.
Facade thinning (an optional follow-up) would raise it further without changing the goal metric.

## Optional follow-ups

Expand Down
74 changes: 74 additions & 0 deletions scripts/kotlin-migration-facades.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Java files that stay Java by design for the duration of the migration.
#
# These declare the public API that customers and kit authors compile against
# (see docs/kotlin-migration/PLAYBOOK.md, "Scope"), plus package-info.java,
# which has no Kotlin equivalent. scripts/kotlin_migration_progress.py counts
# them separately so that "Java left to convert" can honestly reach zero.
#
# Adding a file here is an API-surface decision that needs a maintainer's
# review, not just the author's. One repository-relative path per line.
android-core/src/main/java/com/mparticle/AttributionError.java
android-core/src/main/java/com/mparticle/AttributionListener.java
android-core/src/main/java/com/mparticle/AttributionResult.java
android-core/src/main/java/com/mparticle/BaseEvent.java
android-core/src/main/java/com/mparticle/Configuration.java
android-core/src/main/java/com/mparticle/MPEvent.java
android-core/src/main/java/com/mparticle/MPReceiver.java
android-core/src/main/java/com/mparticle/MPService.java
android-core/src/main/java/com/mparticle/MParticle.java
android-core/src/main/java/com/mparticle/MParticleOptions.java
android-core/src/main/java/com/mparticle/MParticleTask.java
android-core/src/main/java/com/mparticle/SdkListener.java
android-core/src/main/java/com/mparticle/Session.java
android-core/src/main/java/com/mparticle/commerce/CommerceEvent.java
android-core/src/main/java/com/mparticle/commerce/Impression.java
android-core/src/main/java/com/mparticle/commerce/Product.java
android-core/src/main/java/com/mparticle/commerce/Promotion.java
android-core/src/main/java/com/mparticle/commerce/TransactionAttributes.java
android-core/src/main/java/com/mparticle/commerce/package-info.java
android-core/src/main/java/com/mparticle/consent/CCPAConsent.java
android-core/src/main/java/com/mparticle/consent/ConsentInstance.java
android-core/src/main/java/com/mparticle/consent/ConsentState.java
android-core/src/main/java/com/mparticle/consent/GDPRConsent.java
android-core/src/main/java/com/mparticle/consent/package-info.java
android-core/src/main/java/com/mparticle/identity/AliasRequest.java
android-core/src/main/java/com/mparticle/identity/AliasResponse.java
android-core/src/main/java/com/mparticle/identity/BaseIdentityTask.java
android-core/src/main/java/com/mparticle/identity/IdentityApi.java
android-core/src/main/java/com/mparticle/identity/IdentityApiRequest.java
android-core/src/main/java/com/mparticle/identity/IdentityApiResult.java
android-core/src/main/java/com/mparticle/identity/IdentityHttpResponse.java
android-core/src/main/java/com/mparticle/identity/IdentityStateListener.java
android-core/src/main/java/com/mparticle/identity/MParticleUser.java
android-core/src/main/java/com/mparticle/identity/TaskFailureListener.java
android-core/src/main/java/com/mparticle/identity/TaskSuccessListener.java
android-core/src/main/java/com/mparticle/identity/package-info.java
android-core/src/main/java/com/mparticle/internal/package-info.java
android-core/src/main/java/com/mparticle/media/MPMediaAPI.java
android-core/src/main/java/com/mparticle/media/MediaCallbacks.java
android-core/src/main/java/com/mparticle/media/package-info.java
android-core/src/main/java/com/mparticle/messaging/InstanceIdService.java
android-core/src/main/java/com/mparticle/messaging/MPMessagingAPI.java
android-core/src/main/java/com/mparticle/messaging/MPMessagingRouter.java
android-core/src/main/java/com/mparticle/messaging/MessagingConfigCallbacks.java
android-core/src/main/java/com/mparticle/messaging/ProviderCloudMessage.java
android-core/src/main/java/com/mparticle/messaging/PushAnalyticsReceiver.java
android-core/src/main/java/com/mparticle/messaging/PushAnalyticsReceiverCallback.java
android-core/src/main/java/com/mparticle/messaging/package-info.java
android-core/src/main/java/com/mparticle/networking/BaseNetworkConnection.java
android-core/src/main/java/com/mparticle/networking/Certificate.java
android-core/src/main/java/com/mparticle/networking/DomainMapping.java
android-core/src/main/java/com/mparticle/networking/MPConnection.java
android-core/src/main/java/com/mparticle/networking/MPUrl.java
android-core/src/main/java/com/mparticle/networking/NetworkOptions.java
android-core/src/main/java/com/mparticle/package-info.java
android-core/src/main/java/com/mparticle/segmentation/Segment.java
android-core/src/main/java/com/mparticle/segmentation/SegmentListener.java
android-core/src/main/java/com/mparticle/segmentation/SegmentMembership.java
android-core/src/main/java/com/mparticle/segmentation/package-info.java
android-kit-base/src/main/java/com/mparticle/kits/CommerceEventUtils.java
android-kit-base/src/main/java/com/mparticle/kits/FilteredIdentityApiRequest.java
android-kit-base/src/main/java/com/mparticle/kits/FilteredMParticleUser.java
android-kit-base/src/main/java/com/mparticle/kits/KitIntegration.java
android-kit-base/src/main/java/com/mparticle/kits/KitUtils.java
android-kit-base/src/main/java/com/mparticle/kits/ReportingMessage.java
Loading
Loading