Skip to content

chore: normalize project.pbxproj ordering - #1705

Closed
nan-li wants to merge 2 commits into
mainfrom
nan/jwt-pr1-path-encoding
Closed

chore: normalize project.pbxproj ordering#1705
nan-li wants to merge 2 commits into
mainfrom
nan/jwt-pr1-path-encoding

Conversation

@nan-li

@nan-li nan-li commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

One Line Summary

One-time canonical sort of project.pbxproj object sections so later stacked PRs show only their own file additions, plus a pinned xcodeproj gem and CI guard to keep that baseline clean.

Details

Motivation

The xcodeproj gem re-sorts the project file into UUID order on save. main already has ~126 lines of pre-existing disorder relative to that order, so the first PR that uses the gem would otherwise bury real changes in reorder noise. Landing the normalization alone keeps follow-up PRs readable.

Scope

  • iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj reorder only (no source or behavior changes)
  • Root Gemfile / Gemfile.lock pinning xcodeproj 1.28.1
  • CI workflow that asserts a gem save of OneSignal.xcodeproj is a no-op
  • Does not gem-normalize examples/demo/App.xcodeproj (XcodeGen-owned)

Merge note

Land this before #1703, then rebase #1703 onto the normalized baseline.

Testing

Unit testing

Not needed for the reorder — no compile inputs change.

Manual testing

Opened the project with the pinned xcodeproj gem and confirmed a no-op save after this commit is empty of further churn.

CI

pbxproj-canonical runs Project.open(...).save + git diff --exit-code on OneSignal.xcodeproj.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

Made with Cursor

Canonical sort applied by the xcodeproj tool, so later commits in this
stack show only their own file additions. No project changes.

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Multimodal adversarial review (interrogate)

Intent

Canonicalize iOS_SDK/OneSignalSDK/OneSignal.xcodeproj/project.pbxproj to the Ruby xcodeproj gem’s save order so later stacked PRs aren’t buried in ~126 lines of pre-existing reorder noise. Scoped as reorder-only: no source/behavior/build-setting value changes.

Reviewers

  • Reviewer A: claude-fable-5-thinking-xhigh, 2 findings
  • Reviewer B: gpt-5.6-sol-xhigh, 0 findings
  • Reviewer C: cursor-grok-4.5-high-fast, 0 findings
  • Reviewer D: claude-opus-5-thinking-high, 4 findings

All four independently confirmed the pure-reorder claim (identical 9814-line multiset; gem 1.28.1 round-trip of base → head is byte-identical; head save is a no-op).

Act On

None against this diff. The change is a verified content-preserving reorder of dictionary keys / object-map entries and achieves the stated, file-scoped intent.

Consider

  1. Landing race with open PR #1703 (D) — #1703 (fadi/sdk-4977) inserts 4977… objects next to the exact 3C14E3A9…AE block this PR relocates, at non-canonical UUID positions. If #1703 merges from current main without rebasing onto this baseline, the next gem save regenerates the same class of noise this PR exists to eliminate, and the overlapping hunks are merge-hostile. Agree merge order: land this first, then rebase #1703 (or normalize its inserts).
  2. Pin / record the xcodeproj gem version (D) — repo has no Gemfile. Head is a fixed point on 1.26–1.28; older gems (e.g. 1.25) still rewrite HEADER_SEARCH_PATHS quoting. Without a pin, “canonical” is not reproducible across machines.
  3. No enforcement that the baseline stays clean (A, D) — the disorder being fixed was introduced by parent #1702. A tiny CI check (Xcodeproj::Project.open(...).save + git diff --exit-code on OneSignal.xcodeproj) would protect the motivation. Scope carefully: examples/demo/App.xcodeproj is XcodeGen-generated and should not be gem-normalized.
  4. Sibling project drift outside this PR’s scope (A) — OneSignalExample.xcodeproj is not a no-op under gem save (contentful empty-array deletions observed). Fine to leave out of this chore given the explicit single-file scope; worth a follow-up if stacked work will touch the dev app via the gem.
  5. Branch / stack hygiene (D) — this reorder-only chore lives on nan/jwt-pr1-path-encoding, and #1706 is already based on it. Prefer a dedicated chore/pbxproj-normalizemain if the branch tip may later gain path-encoding commits after review.

Noted

  • Pre-existing 23-hex-character object IDs from #1702 (A, D): gem preserves them; not introduced by this reorder. Optional follow-up to regenerate to 24-char form while the file is quiet.

Dismissed

  • Normalize examples/demo/App.xcodeproj in this PR (A) — demo project is generated by xcodegen (see examples/demo/build.md / .github/actions/setup-demo). Gem-normalizing it fights the generator.
  • Any correctness / build-setting-value regressions in this diff (implicit adversary scan) — line multiset identity + gem round-trip disprove content drift; ordered arrays (children, files, targets, etc.) were not reordered.

Agreement Map

  • Strong consensus (4/4): pure reorder, intent achieved for OneSignal.xcodeproj.
  • Divergence: A/D pushed process/sustainability findings (sibling projects, CI, gem pin, #1703 race, branch naming); B/C returned empty reviews after verifying the multiset. Pattern fits a mechanical chore — quality risk is recurrence after merge, not the patch itself.

Verdict: Safe to merge as a no-behavior baseline, with explicit landing coordination vs #1703 and a follow-up for gem pin / CI guard if the gem will keep writing this file.

Open in Web View Automation 

Sent by Cursor Automation: Untitled

Comment on lines +1347 to +1352
3C14E3A92FAE54C006ED053 /* IOSLogger.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IOSLogger.swift; sourceTree = "<group>"; };
3C14E3AA2FAE54C006ED053 /* OneSignalLogHttpSender.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OneSignalLogHttpSender.swift; sourceTree = "<group>"; };
3C14E3AB2FAE54C006ED053 /* FileLogStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileLogStore.swift; sourceTree = "<group>"; };
3C14E3AC2FAE54C006ED053 /* OSLoggerPlatformProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSLoggerPlatformProvider.swift; sourceTree = "<group>"; };
3C14E3AD2FAE54C006ED053 /* KotlinByteArray+Data.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KotlinByteArray+Data.swift"; sourceTree = "<group>"; };
3C14E3AE2FAE54C006ED053 /* OSLoggerAdaptersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OSLoggerAdaptersTests.swift; sourceTree = "<group>"; };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider (Reviewer D; lead agrees): This relocated 3C14E3A9…AE PBXFileReference cluster is the same neighborhood open PR #1703 (fadi/sdk-4977) inserts into with non-canonical 4977… IDs.

The reorder here is verified content-preserving. The risk is landing order: if #1703 merges from current main without rebasing onto this baseline, the next xcodeproj gem save regenerates the reorder noise this PR exists to eliminate (and the overlapping hunks are merge-hostile).

Please agree merge sequence with #1703 — land this first, then rebase #1703 (or have #1703 emit gem-canonical inserts).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed — land this PR first, then rebase #1703 onto the normalized baseline (or have it emit gem-canonical inserts) so the next xcodeproj save doesn’t regenerate reorder noise.

Comment on lines +5440 to +5447
"FRAMEWORK_SEARCH_PATHS[sdk=iphoneos*]" = (
"$(inherited)",
"$(SRCROOT)/../../OneSignal-KMP-SDK/kmp/build/XCFrameworks/release/OneSignalKMP.xcframework/ios-arm64",
);
"FRAMEWORK_SEARCH_PATHS[sdk=iphonesimulator*]" = (
"$(inherited)",
"$(SRCROOT)/../../OneSignal-KMP-SDK/kmp/build/XCFrameworks/release/OneSignalKMP.xcframework/ios-arm64_x86_64-simulator",
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider (Reviewers A/D): These FRAMEWORK_SEARCH_PATHS keys only moved within buildSettings (order-insensitive) — values are unchanged, and gem 1.28.1 round-trips head as a no-op.

Two sustainability gaps remain outside this hunk:

  1. No Gemfile pin — older xcodeproj (e.g. 1.25) still rewrites HEADER_SEARCH_PATHS quoting, so “canonical” isn’t reproducible.
  2. No CI guard that Project.open(...).save is a no-op on OneSignal.xcodeproj (the disorder came from parent feat: [SDK-4976] add Swift adapters for KMP logger #1702).

Worth a small follow-up if the gem will keep editing this file; do not gem-normalize examples/demo/App.xcodeproj (XcodeGen-owned).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in this PR:

  1. Root Gemfile / Gemfile.lock pins xcodeproj to 1.28.1.
  2. New pbxproj-canonical workflow opens/saves OneSignal.xcodeproj and fails on any diff.

examples/demo/App.xcodeproj is intentionally left alone (XcodeGen-owned).

Keep the reorder baseline reproducible and stop the next gem save from
quietly reintroducing disorder.

Co-authored-by: Cursor <cursoragent@cursor.com>
@nan-li nan-li closed this Aug 12, 2026
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.

1 participant