Conversation
|
This pull request requires reviews from CODEOWNERS as it changes files that match the following patterns:
|
|
Preview URL: https://16c32c57.preview.developers.cloudflare.com Files with changes (up to 15) |
e273f53 to
f4838ce
Compare
|
Changelog file is added manually. |
8d8a06f to
d7b620b
Compare
|
/bonk review this changelog entry and recommend how it could be improved |
|
Here is my review of the changelog entry with specific recommendations for improvement: Changelog Review:
|
d7b620b to
a05fabd
Compare
|
Updated based on the review. Updated based on the latest internal repo, too. |
a05fabd to
a9b9cc3
Compare
There was a problem hiding this comment.
Review Summary
The changelog is well-structured after the earlier review round — good improvements there. The field additions across the dataset files look correct and are properly placed in alphabetical order within each file. The new MCP Portal Logs dataset file is well-formed. The Email Security Alerts capitalization fixes and the ProxyEndpoint type/description fix in gateway_http.md are good cleanups.
A few issues to address:
1. Bare URLs in dex_application_tests.md (inconsistency)
The two new fields HTTPRedirectEndMs and HTTPRedirectStartMs use bare URLs:
Refer to https://developer.mozilla.org/en-US/docs/Web/API/Resource_Timing_API/Using_the_Resource_Timing_API for more details.
Every other field in the same file uses a proper markdown link:
Refer to [Resource timing](https://developer.mozilla.org/en-US/docs/Web/API/Resource_Timing_API/Using_the_Resource_Timing_API) for more details.
These should be updated to use [Resource timing](URL) for consistency.
2. Use "for example" instead of "e.g."
The Cloudflare docs style guide prefers "for example" over "e.g." Two new field descriptions use "e.g.":
network_analytics_logs.md→DNSQueryType: "The DNS query type (e.g., A, AAAA, MX, TXT)"http_requests.md→PayPerCrawlStatus: "Pay Per Crawl outcome, when applicable (e.g. request enabled for charging...)"
These should be changed to "for example" for style guide compliance. (Note: I understand these are synced from an internal repo's templates, so this may need to be fixed at the source.)
3. Minor: note on code-generated files
Since these files are marked # Code generator. DO NOT EDIT., the bare URL and "e.g." issues likely originate from the internal templates. If fixing at the source is not feasible, it would still be worth patching them here so the published docs are consistent.
Everything else looks good — the changelog covers all the new fields accurately, the field ordering is correct, and the frontmatter is valid.
src/content/docs/logs/logpush/logpush-job/datasets/account/dex_application_tests.md
Outdated
Show resolved
Hide resolved
src/content/docs/logs/logpush/logpush-job/datasets/account/dex_application_tests.md
Outdated
Show resolved
Hide resolved
src/content/docs/logs/logpush/logpush-job/datasets/account/network_analytics_logs.md
Show resolved
Hide resolved
src/content/docs/logs/logpush/logpush-job/datasets/zone/http_requests.md
Show resolved
Hide resolved
|
Review posted on PR #28296. Here's a summary of what I found: Overall: The PR is in good shape. The changelog was improved well based on the earlier review. The new MCP Portal Logs dataset and all field additions are correctly structured and alphabetically ordered. The Email Security capitalization fixes and 3 issues flagged with inline suggestions:
All three have concrete |
|
Updated based on Updated to the latest from the internal repo. Updated changelog with the new date. |
a9b9cc3 to
e8dc45c
Compare
There was a problem hiding this comment.
All previously flagged issues have been addressed in the latest update:
- Bare URLs in
dex_application_tests.md— Fixed.HTTPRedirectEndMsandHTTPRedirectStartMsnow use[Resource timing](URL)markdown links, consistent with the rest of the file. - "e.g." → "for example" — Fixed in
network_analytics_logs.md(DNSQueryType),http_requests.md(PayPerCrawlStatus),dex_device_state_events.md(3 fields), anddlp_forensic_copies.md(DataPhase). - Email Security capitalization — Fixed across
email_security_alerts.md. ProxyEndpointtype/description ingateway_http.md— Fixed (now hasstringtype and a proper description).
The changelog is well-structured, accurately lists all new fields and the new MCP Portal Logs dataset, and includes a link to the full dataset documentation. The new mcp_portal_logs.md file is properly formatted with fields in alphabetical order and consistent frontmatter.
New fields added in this update (AppControlInfo, PFPCustomTag, SNI, etc.) are also reflected in the changelog. The changes beyond the previous review round (additional fields from the latest internal repo sync, updated changelog date to 2026-03-09) all look correct.
LGTM.
|
Review posted and PR approved. Here's a summary of what I found: All 4 previously flagged issues have been resolved:
No new issues found. The changelog is accurate and well-structured, the new MCP Portal Logs dataset file is properly formatted, all fields are in alphabetical order, and frontmatter is consistent with the existing files. The PR is in good shape to merge. |
This updates Log fields. They are synced from internal `entities` repo's fields templates. Changelog file is added manually.
e8dc45c to
16c32c5
Compare
Summary
This updates Log fields.
They are synced from internal
entitiesrepo's fields templates.Changelog file is added manually.
Documentation checklist