Repository navigation
Developer guide: restore 19 dropped code examples as compiled snippets - #5711
Merged
Merged
Conversation
bbdc605 converted inline listings into include:: directives and left the introducing sentence, its colon and two blank lines where the code had been. This restores them for five chapters, verbatim from bbdc605~1, as compiled snippets in the demos module. Restoring code rather than deleting the promise is what makes this worth doing: every sample now compiles against the current core, so the guide cannot claim an API that isn't there. Three things only a compiler was going to find: - the samples use com.codename1.sensors, com.codename1.car and com.codename1.nfc, none of which the shared snippet preamble imported - "Daft Punk -- Discovery" carried a literal em dash into a .java file, which javac -encoding ascii rejects outright - two NFC bodies were reshaped: a body opening with `new NfcReadOptions()` reads as a method signature to the extractor's classifier, so it was placed at class level and would not parse. A local class inside snippet() is legal Java and keeps the published sample byte-identical. Four holes in these chapters are deliberately left in the baseline, because the code is real but does not belong on this module's classpath: - Deep-Links-Routing 003/004 use com.codename1.maven.routing.AasaBuilder, which lives in the maven plugin. It is build-time tooling, not app code. - Monetization's seven holes are the demo server's receipt validation, which needs the cn1-generic-webservice-client cn1lib and Java EE. Adding either dependency to the demos module to satisfy a doc build would be the tail wagging the dog; those need their own decision. Baseline: 89 -> 70, deletions only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b4e83e68a
ℹ️ 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".
These came back verbatim from bbdc605~1, and restoring them faithfully is not a reason to republish code that does not work. All three were wrong in the original: - the accelerometer sample registered an anonymous MotionSensorListener while the very next sample tells the reader to call removeListener(listener). MotionSensor holds that exact object, so as written the listener could never be removed and the sensor kept drawing power. It is now a named local. - the FeliCa sample built an NfcReadOptions and discarded it. It configured nothing and started no read; the options only take effect when passed to readTag, which is where they now go. - the dirty-form PopGuard threw away the dialog result and returned false unconditionally, so "Discard" behaved exactly like "Stay" and the form could not be left at all. Dialog.show returns true for the first button, so the guard returns its negation. Compiling the restored code found the API mismatches; these three needed reading the sample against what the API does, because each one compiles perfectly and still does not work.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d991a489e
ℹ️ 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".
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
…false
Five more defects in the restored samples, all of which compile:
- the accelerometer listener was a method local, so the removeNotify the next
sample tells you to write cannot reach it. Both the sensor and the listener
are fields now.
- that removal dereferenced a sensor the previous sample explicitly says can
be null, so leaving the form on a device without an accelerometer threw.
- the shake sample registered an anonymous GestureListener with no way to
unregister it. MotionSensorManager keeps the accelerometer powered while any
gesture listener is registered, so the sample now shows both halves.
- the tabletop layout branched on isTableTop(), which is
POSTURE_HALF_OPENED && isSeparating() -- true for book posture on a
vertically hinged device, where a top/bottom split is wrong. It now also
requires FOLD_ORIENTATION_HORIZONTAL.
The last one is not a sample bug at all. The chapter said the Maven plugin and
build daemon "automatically register
com.apple.developer.nfc.readersession.felica.systemcodes from these values".
They do not. IPhoneBuilder injects CoreNFC, a default NFCReaderUsageDescription
and the readersession.formats entitlement when it sees com.codename1.nfc on the
classpath, and neither this repo nor BuildDaemon mentions felica anywhere. Nor
could it: the codes are an argument to setFelicaSystemCodes at runtime, which no
build-time classpath scan can observe. An app built by following that sentence
cannot read FeliCa on iOS.
The chapter now says what to set, using the ios.entitlements.* namespace the
generic renderer already emits. NfcF's javadoc made the same claim in the same
words -- that is where the sentence came from -- so it is corrected too, or the
guide would just inherit it again. TagType's wording was already right.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2ddfe34c3
ℹ️ 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".
Contributor
Cloudflare Preview
|
…list Three things, one of them a mistake I made two commits ago. The FeliCa system codes and the ISO 7816 select identifiers are Info.plist keys, not entitlements. Only readersession.formats is an entitlement. My previous fix told the reader to set the FeliCa codes under ios.entitlements.*, which would put them in the wrong file and still not work. Both are now ios.plistInject, with the plist XML spelled out, in the chapter and in NfcF's javadoc. setIsoSelectAids has exactly the same gap as setFelicaSystemCodes and it was not documented at all: Core NFC only discovers an ISO 7816 application the app declared up front, and no builder derives that key from the runtime call. Note the builders DO handle nfc.hce.iso7816.select-identifiers -- that is the host card emulation key, a different one, and the near miss is worth calling out because grepping for "iso7816" finds it and suggests the case is covered. The head-unit row limit was assigned to a local and never used, while the prose above it says to trim accordingly. It now caps the loop, treating 0 as unknown. Written with an index loop rather than a List local because the snippet fixture imports both java.util and com.codename1.ui.list, so a bare List is ambiguous.
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb4d6ea725
ℹ️ 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".
The chapter told the reader to unregister the sensor listener "from the form's removeNotify". There is no such method: Component has deinitialize() and initComponent(), Form adds hideNotify(). removeNotify is Swing. Following that sentence produces either a compile error with @OverRide or a method nothing ever calls, and either way the listener stays registered and the accelerometer stays powered after the screen goes away. The prose now names deinitialize(), and the samples put the fields somewhere the teardown can actually reach them: registration is initComponent() on a Form subclass, removal is the matching deinitialize(). The posture listener had the same defect for the same reason -- registered as a lambda with nothing holding it, so CN.removePostureListener could never be handed the same instance, and CN registers on the singleton Display, so every recreated screen stayed reachable and kept receiving posture changes. It is a field now, added and removed in the pair of hooks.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40a9eb9b89
ℹ️ 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".
My own fix from three commits ago was wrong in a way that loses user data.
Dialog.show builds its commands as {cancelText, okText} and calls
setBackCommand(cmds[0]), so the hardware back key fires the CANCEL command and
show() returns false. With "Stay" as ok and "Discard" as cancel, plus the
inversion I added, backing out of the dialog returned true and the pop went
ahead -- the user never chose Discard and lost the edits anyway.
Discard is now the ok command and Stay the cancel command, and the result is
returned directly. Dismissal fires Stay, returns false, and the guard blocks the
pop, so the accident-safe outcome is the one the back key produces.
Audited the rest of the batch for both classes rather than waiting to be told.
The other Dialog.show passes null for cancelText, so it always returns true and
its result is unused. Of the listener registrations, three are component-scoped
and die with the component; Car.addConnectionListener is static like the posture
one, so it keeps a reference now. The HCE registration is left alone on purpose:
unregisterHostCardEmulationService takes no argument, so nothing needs holding.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cb2ba5359
ℹ️ 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".
I made this a local variable one commit ago while writing a comment saying it has to be handed back to Car.removeConnectionListener at teardown, which a local cannot be. That is the same field-vs-local mistake I was already corrected on for the accelerometer earlier in this branch, repeated after the correction. It is now a field on a Form subclass with initComponent/deinitialize, matching the accelerometer and posture samples. Car keeps listeners in a static list, so without this every recreated screen stays reachable and keeps receiving connection callbacks. Swept the whole batch for the pattern rather than fixing the one that was reported: all four retained listeners are fields.
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 160 screenshots: 160 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
bbdc6058f0converted inline listings intoinclude::directives and dropped roughly 415 of them, leaving the introducing sentence, its colon and two blank lines where the code had been. This restores 19 of them across five chapters, verbatim frombbdc6058f0~1, as snippets that compile in the demos module.Restoring the code rather than deleting the promise is the point. Every sample now compiles against the current core, so the guide can't claim an API that isn't there. Three defects only a compiler was going to find:
com.codename1.sensors,com.codename1.carandcom.codename1.nfc-- none of which the shared snippet preamble imports;"Daft Punk — Discovery"carried a literal em dash into a.javafile, whichjavac -encoding asciirejects outright;new NfcReadOptions()reads as a method signature to the extractor's shape classifier, so it was emitted at class level and wouldn't parse. A local class insidesnippet()is legal Java and keeps the published sample byte-identical.One near-miss worth recording:
setIsoSelectAids(myAid)failed to compile, which looked like the guide documenting a signature that doesn't exist. It was my scaffolding -- an AID is abyte[], and the real signature issetIsoSelectAids(byte[]...). The sample was right. Checked before "fixing" the guide.Four holes in these chapters are deliberately left in the baseline, because the code is real but doesn't belong on this module's classpath:
com.codename1.maven.routing.AasaBuilder, which lives in the maven plugin -- build-time tooling, not app codecn1-generic-webservice-clientcn1lib and Java EE (@Stateless,EntityManager)Adding either dependency to the demos module to satisfy a doc build would be the tail wagging the dog. Those need their own decision, so they stay recorded as holes rather than being quietly dropped.
Ratchet: 89 → 70, deletions only. Verified:
mvn process-classesondocs/demosbuilds all 685 sources (this needed a core rebuild locally -- the demos tree on master already usesNativeDragAndDropfrom #5662).Disjoint from #5710: different chapters, different baseline file.
🤖 Generated with Claude Code