Developer guide: four more sentences that promise code and do not deliver - #5813
Merged
Conversation
…iver Deep-Link Routing twice says "the plugin's <Builder> produces the payload:" and then shows nothing. Both payloads are now what the builders write: AasaBuilder and AssetLinksBuilder were run against the chapter's own example -- the /users/:id route it documents, a Services ID, two signing fingerprints -- and their output committed. Note addRouterPattern turns /users/:id into /users/*, which is the conversion the chapter describes in prose and now shows. Advertising promised the initialize-then-consent-then-load order and then described AdConfig's flags as if the reader had just seen them. That listing is restored, with the reason the order matters beside it: requestConsent presents the GDPR form and, on iOS, the App Tracking Transparency prompt, and both need the SDK up, so consent sits between initialize and the first load rather than before either. The fourth is not restorable and is rewritten instead. Each provider's install() lives in its own cn1lib, none of which the docs module depends on, so a compiled listing would mean a build dependency for one static call. The sentence now names the four calls and points at the table right above it that gives the class for each. Ratchet drops from 34 to 30. 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: 882ce45d71
ℹ️ 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 Android half of deep linking said to set android.xintent_filter "with a verified intent filter for your domain" and left the reader to work out what that is. The hint's value is injected into the manifest verbatim, so it is the whole element with escaped line breaks -- not something to guess at -- and the three parts that decide whether it works are now spelled out: autoVerify is what makes Android fetch the assetlinks.json beside it rather than show an app chooser, the data element needs the https scheme and host, and the hint holds one value so a second filter goes in the same string. This is also what the Authentication chapter's Apple-on-Android note points at: the Services ID callback is an HTTPS URL, so the custom scheme registered there cannot receive it and this is the recipe that can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AdManager.initialize reports false when no provider was installed or the network's own initialize failed, and the listing ignored the flag. With no provider AdConsent reports consent as not required and canRequestAds() answers true, so the recommended order walked straight into loadAds() with nothing behind it -- the one arrangement where the graceful degradation the chapter describes hides the mistake instead of surfacing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56a8181fd1
ℹ️ 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".
Both are weaker than the chapter implied, and both are in the providers rather than in the listing, so the listing keeps its shape and the chapter now says what it is looking at. The readiness flag is not a report that the network came up. Every provider -- AdMob, AppLovin, LevelPlay -- hands the config to its native bridge and calls back true on the next line, while the SDK behind it initializes asynchronously. An invalid SDK key therefore surfaces as failing loads later, never as false here. And requestConsent presents a form only where the provider implements one. AdMob does, through Google's User Messaging Platform. AppLovin MAX and Unity LevelPlay read consent from an external CMP and the IAB TCF string, so their native requestConsent reports STATUS_NOT_REQUIRED immediately and canRequestAds() always answers true -- which means on those two the EEA/UK consent flow is the CMP's and has to be in place before the first load. A reader following the recommended order without knowing that would have believed the framework was collecting consent it never collects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 115248250b
ℹ️ 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".
On iOS 14+ the AdMob provider starts ATTrackingManager with an empty completion handler and fires its consent callback from the UMP path independently, so the callback can arrive while the system tracking prompt is still on screen. The recommended order in this chapter then loads ads before tracking authorization is settled, which is the one thing that ordering exists to prevent. Documented as a warning with what to do instead. The provider itself needs a change -- the UMP request belongs inside the ATT completion handler -- and that is a cn1-admob fix rather than a docs one. And the App Link paragraph said an unverified filter still matches and asks the user each time. From Android 12 that is not what happens: an unverified web link goes to the default browser and the filter never sees it until the user turns the association on by hand. A reader who skipped autoVerify on the strength of the old sentence would have found deep links silently not working at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
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
1 participant
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.
Four more holes in the
check-missing-code-blocksratchet, in two chapters.Deep-Link Routing — the payloads are generated
The chapter twice says "the plugins
AasaBuilder/AssetLinksBuilderproduces the payload:" and then shows nothing.Both payloads are now what the builders actually write. I ran them against the chapters own example — the
/users/:idroute it documents in its routing table, a Services ID, two signing fingerprints — and committed the output.Worth noticing in the result:
addRouterPattern("/users/:id")emits{ "/": "/users/*" }. That conversion is described in prose two sections earlier and is now visible.Advertising — the consent order
The chapter promised the initialize → consent → load order and then went straight on to describe
AdConfigs compliance flags as though the reader had just seen them.The listing is restored with the reason the order matters beside it:
requestConsentpresents the GDPR form and, on iOS, the App Tracking Transparency prompt, and both need the SDK up — so consent sits betweeninitializeand the first load rather than before either. It also shows thecanRequestAds()branch, because consent withheld is a normal outcome, not an error.Advertising — the one that could not be restored
Each providers
install()lives in its own cn1lib (cn1-admob,cn1-applovin,cn1-unity-levelplay,cn1-ads-mock), none of whichdocs/demosdepends on. A compiled listing would mean taking a build dependency for one static no-arg call.The sentence now names all four calls inline and points at the table directly above it that gives the provider class for each — no information lost, and no promise left unkept.
Gates
check-missing-code-blocksratchet 34 → 30validate-guide-snippets.py— 1110 include-backed blockscodenameone-core(it caught two constant names:AdConfig.TAG_FALSEandRATING_G, not the longer spellings)asciidoctor --failure-level WARNandasciidoctor-pdf— cleanstatus: ok,total: 0on both chapters