Skip to content

NDC 5.8 PostgreSQL documentation - #1619

Open
BenHayman-Netwrix wants to merge 10 commits into
devfrom
NDC/release-5.8
Open

NDC 5.8 PostgreSQL documentation#1619
BenHayman-Netwrix wants to merge 10 commits into
devfrom
NDC/release-5.8

Conversation

@BenHayman-Netwrix

@BenHayman-Netwrix BenHayman-Netwrix commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@BenHayman-Netwrix
BenHayman-Netwrix requested a review from a team as a code owner September 30, 2026 13:28
@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

74 issues fixed, 8 skipped across 10 files

Category Fixes
OnceUsage (rewrite) 1
Dale: misplaced-modifiers 1
Dale: passive-voice 58
Dale: undefined-acronyms 1
Dale: wordiness 13
Skipped (needs manual review) Reason

| docs/dataclassification/5.7/index.md:10 — Dale: idioms | 'unleash the true value of this data' is metaphorical, but it is established product positioning copy on the landing page; a literal rewrite would change the intended marketing meaning |
| docs/dataclassification/5.7/index.md:30 — Dale: idioms | 'without putting your business on hold' is idiomatic, but it sits in a benefits bullet where a literal replacement would change the claim the author is making |
| docs/dataclassification/5.7/index.md:32 — Dale: passive-voice | heading text — not modified to avoid breaking anchor links |
| docs/dataclassification/5.7/index.md:119 — Dale: passive-voice | 'The hitlist itself is always constructed and ranked using information contained in the proprietary conceptDatabase' — the acting component isn't stated and could be the QueryServer or the Indexer; naming the wrong actor would introduce a technical error |
| docs/dataclassification/5.7/introduction/deployment/ndcserverandclient/dqsmode.md:96 — Dale: misplaced-modifiers | 'When upgrading an existing NDC instance the installer will attempt to resynchronize...' reads as dangling, but the installer is in fact the agent performing the upgrade, so the modifier attaches correctly |
| docs/dataclassification/5.7/introduction/deployment/ndcserverandclient/dqsmode.md:33 — Dale: wordiness | 'If you want to implement DQS configuration for your NDC deployment, be aware that you can't undo it' — the conditional framing carries a deliberate warning emphasis that a tighter rewrite would weaken |
| docs/dataclassification/5.7/systemconfigurationoverview/configuration/coreconfiguration/system.md:9 — Dale: wordiness | 'describes what the setting does and how it works' is mildly redundant, but the two clauses may describe distinct popup content; trimming risks dropping real information |
| docs/dataclassification/5.7/introduction/upgrade.md:49 — Dale: wordiness | 'After taking the preceding preparatory steps' is slightly redundant, but the phrasing is unambiguous and any rewrite would be a stylistic preference rather than a clear violation |

Ask @claude on this PR if you'd like an explanation of any fix.

@jth-nw
jth-nw deployed to development September 30, 2026 13:49 — with GitHub Actions Active
@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

17 issues fixed, 7 skipped across 10 files

Category Fixes
FollowTheStepsTo (rewrite) 1
Dale: idioms 2
Dale: misplaced-modifiers 1
Dale: passive-voice 7
Dale: undefined-acronyms 1
Dale: wordiness 5
Skipped (needs manual review) Reason

| docs/dataclassification/5.7/index.md:21 — Dale: idioms | 'a single panoramic view of your enterprise content' — the metaphor carries the meaning (one unified view across all stores); a literal replacement risks changing what the sentence claims |
| docs/dataclassification/5.7/index.md:32 — Dale: wordiness | heading text — not modified to avoid breaking anchor links |
| docs/dataclassification/5.7/index.md:113 — Dale: passive-voice | 'the queue of documents being indexed' is an established noun phrase in this product's terminology; rewriting would not improve clarity |
| docs/dataclassification/5.7/introduction/deployment/ndcserverandclient/dqsmode.md:20 — Dale: passive-voice | 'Each clustered NDC Server will store its own set of .CSE index files' is already active; the surrounding future-tense phrasing is a tense issue, not a Dale rule |
| docs/dataclassification/5.7/systemconfigurationoverview/configuration/coreconfiguration/system.md:23 — Dale: passive-voice | 'Disabled by default. When enabled, ...' appears in a terse table cell; an active rewrite would add a subject that breaks the table's compact style |
| docs/dataclassification/5.7/introduction/requirements/softwarerequirements.md:12 — Dale: positional-references | 'Windows 2012 R2 and above' refers to a version range, not to content positioned above on the page |
| docs/dataclassification/5.7/introduction/upgrade.md:53 — Dale: positional-references | 'an earlier version' refers to a product version, not to content earlier on the page |

Ask @claude on this PR if you'd like an explanation of any fix.

@jth-nw
jth-nw deployed to development September 30, 2026 14:10 — with GitHub Actions Active
graham-simms-nwx
graham-simms-nwx reviewed Sep 30, 2026
View reviewed changes
Comment thread docs/dataclassification/5.7/introduction/deployment/clouddeployment.md

@graham-simms-nwx graham-simms-nwx Sep 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should document that we support PostgreSQL version 16 and higher. I have done all my testing on 16, and that is the hardened version available to the DoD - so USAF friendly. NB It should actually work on 13+ as that is the oldest version containing all the functionality we use.

Comment thread docs/dataclassification/5.7/introduction/requirements/softwarerequirements.md

@graham-simms-nwx graham-simms-nwx Sep 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

At some point we need to increase the supported Windows/SQL versions Windows 2012 and SQL 2008 are kind of crazy. Again the PostgreSQL here should be 16

@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Documentation PR Review

This PR does two things at once: it adds PostgreSQL as a supported database back end, and it applies a broad pass of voice/clarity edits across the NDC 5.7 introduction topics. The voice edits are strong — active voice, imperative steps, and the removal of "will be"/"is implemented as" constructions are all improvements. The findings below concentrate on the PostgreSQL additions, where a few places still assume SQL Server is the only option, plus two markdown structure issues that will render incorrectly.

Editorial Review

docs/dataclassification/5.7/introduction/upgrade.md

  • Completeness — Line 22: For SQL Server: introduces a database-specific branch, but no PostgreSQL branch follows. Since this PR makes PostgreSQL a supported back end, a reader running PostgreSQL reaches Step 2 ("Back up NDC SQL database") and is given no way to complete it. Suggested fix: add a parallel For PostgreSQL: list after the SQL Server bullets, for example: "- Connect to the server hosting the PostgreSQL instance. - Run pg_dump against the NDC database, or use the Backup dialog in pgAdmin. - Wait for the process to complete." If PostgreSQL backup guidance isn't available yet, at minimum link to the PostgreSQL backup documentation so the step is actionable.

docs/dataclassification/5.7/introduction/requirements/softwarerequirements.md

  • Structure — Lines 45–46: there's no blank line between 3. **Database** and **SQL Server**, so markdown treats them as one paragraph and renders them on a single line ("Database SQL Server"). Compare item 2, which has a blank line before **.NET Framework Features**. Suggested fix: insert a blank line after 3. **Database** (and confirm the blank line before **PostgreSQL** on line 50 is preserved).
  • Completeness — Line 53: the note "Large environments may require SQL Server Enterprise edition" now sits under a heading that covers both engines, leaving PostgreSQL readers unsure whether an equivalent constraint applies to them. Suggested fix: scope the note explicitly — "For SQL Server deployments, large environments may require SQL Server Enterprise edition" — and add a sentence on whether PostgreSQL has any equivalent edition or configuration requirement at that scale.
  • Completeness — Line 51: item 1 of this topic restricts the product to Windows Server, but it isn't stated whether the PostgreSQL instance itself must run on Windows or can run on Linux. This is the first question a PostgreSQL-shop reader will ask. Suggested fix: add a bullet such as "The PostgreSQL instance can run on any platform reachable over the network from the NDC Server" (or state the actual restriction).

docs/dataclassification/5.7/introduction/install/ndcsqldatabase.md

  • Structure — Line 70: the paragraph "Netwrix recommends these values for a fully loaded instance…" starts at the left margin between steps 2 and 3, which terminates the numbered list and starts a new one at step 3. Suggested fix: indent the paragraph four spaces so it becomes part of step 2 and the procedure stays a single list.
  • Clarity — Line 72: "Reload the configuration in your PostgreSQL instance by restarting the service" conflates two distinct PostgreSQL operations — a reload (pg_ctl reload / SELECT pg_reload_conf()) and a service restart. A reader familiar with PostgreSQL won't know which you intend. Suggested fix: pick one and name it — "Restart the PostgreSQL service to apply the new configuration."
  • Structure — Line 53: the pg_wal sizing guidance is a capacity prerequisite, not a configuration step, and it appears with no lead-in ahead of the account note and the procedure. Suggested fix: give it a lead-in that marks it as pre-work — "Before you configure the database, allow for a write-ahead log (pg_wal) directory of at least 32 GB…" — and link "Distributed Query Server" to the DQS topic so readers can size correctly.
  • Completeness — Lines 55–58: the note tells the reader which account to use "to create the NDC SQL Database," but the steps in this section only edit postgresql.conf — nothing here creates a database. The reader is left unsure where that account requirement applies. Suggested fix: move the note to the introduction (next to the pgAdmin reference on line 12), where database creation is actually discussed. Also lowercase "Database" to match "NDC SQL database" used everywhere else in the PR.
  • Clarity — Lines 65–68: the table values use 16GB/4GB/15min with no space, which is inconsistent with the "32 GB"/"128 GB" spacing used on line 53 and throughout the requirements topics. Suggested fix: use "16 GB", "4 GB", "15 min", and format the setting names as code (max_wal_size) since they're literal entries in postgresql.conf.

docs/dataclassification/5.7/introduction/requirements/hardwarerequirements.md

  • Completeness — Line 43: the ## SQL Server heading and the phrase "for an SQL database" still frame this whole sizing section as SQL Server-only, but the table at line 133 now names PostgreSQL as a supported option. A PostgreSQL reader can't tell whether these CPU/RAM/disk figures apply to them. Suggested fix: rename the heading to "Database Server" and open with "The requirements in this section apply to the NDC SQL database, whether hosted on SQL Server or PostgreSQL, supporting up to 4 NDC Server instances…"
  • Clarity — Line 133: the three columns of the new Database Requirements row are punctuated three different ways ("SQL Server, or PostgreSQL.", "SQL Server or PostgreSQL", "SQL Server, or PostgreSQL."), and "Standard or Enterprise" reads as though it qualifies PostgreSQL too. Suggested fix: use the same phrasing in all three cells — "SQL Server (Standard or Enterprise edition) or PostgreSQL. 8 cores CPU, …".
  • Clarity — Line 132: "SQL Instance" / "SQL Instances" is now ambiguous, since the instance may be PostgreSQL. The plural is also wrong — each row describes one database server. Suggested fix: "2 servers total: database instance + NDC Server", "3 servers total: database instance + 2 clustered NDC Servers", and so on.
  • Clarity — Line 122: "Each installation requires its own SQL instance" has the same ambiguity on an added line. Suggested fix: "Each installation requires its own database instance."

docs/dataclassification/5.7/introduction/install/overview.md

  • Clarity — Line 16: "select either SQL Server or Postgres" uses "Postgres", while every other page in this PR says "PostgreSQL". Suggested fix: if the installer label really is "Postgres", make that explicit — "select either SQL Server or Postgres (PostgreSQL)" — otherwise use "PostgreSQL" for consistency.
  • Completeness — Line 22: the Server Name example is SQL Server-only ("WORKSTATIONSQL\SQLSERVER"), so a PostgreSQL reader has no model for what to enter (host name, host:port, FQDN?). Netwrix standards call for an example per concept. Suggested fix: add a second example — "For PostgreSQL, use the host name or IP address, for example "ndc-postgres:5432"."
  • Clarity — Line 23: "For SQL Server, choose between Windows and Database authentication" renames what the previous text called "SQL Server authentication" to "Database authentication". Confirm this matches the installer's actual labels, and tighten the second sentence: "PostgreSQL supports Database authentication only."
  • Clarity — Line 28: "click Create to create the SQL Database" still reads as SQL Server-specific and uses inconsistent capitalization. Suggested fix: "click Create to create the database before moving to the next step."

docs/dataclassification/5.7/introduction/deployment/clouddeployment.md

  • Structure — Lines 25–30: the "For all databases:" lead-in is followed by a single bullet, and that bullet ends with a caveat that applies only to SQL Server Express. Content labeled "all databases" that is in fact engine-specific will mislead PostgreSQL readers. Suggested fix: keep the disk space estimate under "For all databases:", and move the SQL Server Express limitation up as a sub-bullet under the SQL Server minimum-version bullet on line 21.
  • Structure — Lines 19–25: minimum supported versions for both engines now appear here, in Software Requirements, and in index.md. Three copies will drift. Suggested fix: state the versions once in Software Requirements and link to it from here, keeping this topic focused on sizing.
  • Clarity — Lines 50–51: "Scalability and performance testing groups environments by the number of objects to classify" makes "testing" the actor performing the grouping, which reads awkwardly. Suggested fix: "The following table groups environments by the number of objects to classify:"
  • Structure — Lines 19, 22–24: trailing whitespace on lines 19 and 22, plus two blank lines before "For all databases:", produce an uneven gap. Suggested fix: trim the trailing spaces and reduce to a single blank line.

docs/dataclassification/5.7/introduction/deployment/ndcserverandclient/dqsmode.md

  • Completeness — Line 100: "The remaining steps resynchronize the NDC instance with the primary NDC server" drops the instruction the original sentence carried. The reader is told the step appears on failure but not what to do — and "the remaining steps" is ambiguous inside a note that just pointed them to step 13. Suggested fix: "The DQS Synchronization step displays for an upgrade only if this process fails. In that case, follow steps 9–12 to resynchronize the NDC instance with the primary NDC server."
  • Clarity — Lines 13–16: "16M"/"64M"/"8M"/"32M" are inconsistent with the "16 M objects" and "16,000,000" formats used in Hardware Requirements and Data Storages and Sizing. Suggested fix: match the sibling topics — "Use DQS mode when processing over 16,000,000 objects. A cluster of 4 servers supports up to 64,000,000 objects."
  • Clarity — Line 31: "You configure DQS mode in the administrative web console and, from version 5.7.10 onwards, the installer" makes the reader carry "in" across the interruption. Suggested fix: "You configure DQS mode in the administrative web console, or, from version 5.7.10 onwards, in the installer."

docs/dataclassification/5.7/index.md

  • Clarity — Lines 119–120: the added text introduces both "hitlist" and "conceptDatabase" without definition, and "conceptDatabase" is a fourth name for storage in a page that already uses "NDC Index", "NDC Index Database", and "NDC SQL Database". Newer readers won't map these to anything. Suggested fix: use the established name — "The QueryServer always constructs and ranks the result list (hitlist) using information contained in the NDC Index." If conceptDatabase is genuinely a distinct component, define it on first use.
  • Completeness — Lines 132–135: the note tells the reader that Netwrix Data Classification doesn't supply "text.cse", but "text.cse" appears nowhere else on the page — only the generic ".cse" extension is mentioned. The reader has no idea what this file is or why its absence matters. Suggested fix: identify it — "Netwrix Data Classification doesn't supply the primary index file, text.cse; it creates the file in the NDC Index directory when the Collector collects the first documents."

docs/dataclassification/5.7/systemconfigurationoverview/configuration/coreconfiguration/system.md

  • Clarity — Line 23: the em dash in "a small amount of information about how you use the product — to improve the functionality of the product and future offerings" separates a purpose clause from the verb it modifies and isn't doing any work. Suggested fix: "the program sends a small amount of information about how you use the product, which Netwrix uses to improve the product and future offerings."
  • Clarity — Line 25: the Comment column was correctly generalized to "the database", but the Description column still reads "the max size (in GB) the SQL database will grow to" — passive-adjacent future tense inconsistent with the rest of the pass, and SQL-specific wording in a PR that generalizes to PostgreSQL. Suggested fix: "Specifies the maximum size (in GB) of the NDC SQL database."

docs/dataclassification/5.7/introduction/deployment/checklist.md

  • No issues found. The step 2 revision resolves the dangling "instance for." fragment and correctly names both engines.

Summary

21 editorial suggestions across 9 files. Two are rendering issues that will be visible on the published page (softwarerequirements.md line 45, ndcsqldatabase.md line 70), and one is a content gap a PostgreSQL customer will hit during an upgrade (upgrade.md line 22). The rest cluster around terminology consistency — "SQL"/"SQL Server"/"Postgres"/"PostgreSQL" and "SQL instance" vs "database instance" — now that the back end is no longer always SQL Server. Vale and Dale issues are auto-fixed separately.

What to do next:

Comment @claude on this PR followed by your instructions to get help:

  • @claude fix all issues — fix all editorial issues
  • @claude help improve the flow of this document — get writing assistance
  • @claude explain the voice issues — understand why something was flagged

You can ask Claude anything about the review or about Netwrix writing standards.

Automated fixes are only available for branches in this repository, not forks.

@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

10 issues fixed, 6 skipped across 10 files

Category Fixes
OnceUsage (rewrite) 1
Dale: misplaced-modifiers 1
Dale: passive-voice 4
Dale: wordiness 4
Skipped (needs manual review) Reason

| docs/dataclassification/5.7/index.md:32 — Dale: n/a | heading text — not modified to avoid breaking anchor links ("# How It Works" is a second H1 on the page, but renaming or re-leveling a heading breaks inbound anchors) |
| docs/dataclassification/5.7/index.md:113 — Dale: passive-voice | "the queue of documents being indexed" is passive, but the two candidate rewrites change the meaning: "documents awaiting indexing" (matching the NDC Collector section) describes a queue the Indexer hasn't reached yet, while the current wording may intentionally cover documents already in flight. Not confident which is accurate. |
| docs/dataclassification/5.7/index.md:133 — Dale: xy-slop | "Netwrix Data Classification doesn't supply "text.cse" — it creates the file automatically" matches the negative-positive shape, but the negative half carries real information (don't go looking for a shipped file). Collapsing it to the positive statement alone would drop that. |
| docs/dataclassification/5.7/systemconfigurationoverview/configuration/coreconfiguration/system.md:9 — Dale: misplaced-modifiers | The relative clause "which describes what the setting does" is separated from "popup" by a parenthetical. Every rewrite I considered either kept the same separation or required asserting an interaction the source doesn't state (e.g. that you click the "i" symbol to open the popup). |
| docs/dataclassification/5.7/introduction/requirements/hardwarerequirements.md:57 — Dale: wordiness | "Ensure that your ... servers are available" could drop "that", but the change is mechanical style that Vale owns rather than a Dale wordiness violation. |
| docs/dataclassification/5.7/introduction/deployment/clouddeployment.md:16 — Dale: n/a | heading text — not modified to avoid breaking anchor links (page jumps from H1 to "### NDC SQL database", skipping a level, but re-leveling changes nothing about the anchor and the fix belongs with the author) |

Ask @claude on this PR if you'd like an explanation of any fix.

@jth-nw
jth-nw deployed to development September 30, 2026 14:42 — with GitHub Actions Active
@DanPiazza-Netwrix Claude Code
Move NDC 5.8 changes to new 5.8 docs version
bf6c215 
Revert 5.7 pages to match dev and add a 5.8 version containing the
PostgreSQL and 5.8 updates. Copy 5.7 images to a 5.8 image folder and
point 5.8 image and internal links at 5.8. Add the 5.8 sidebar and make
5.8 the latest/default Data Classification version.

Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
@DanPiazza-Netwrix Claude Code
Update NDC 5.8 upgrade path wording
d909982 
Generated with AI

Co-Authored-By: Claude Code <ai@netwrix.com>
@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

222 issues fixed, 19 skipped across 206 files

Category Fixes
FirstPersonPlural (rewrite) 1
Dale: minimizing-difficulty 1
Dale: misplaced-modifiers 2
Dale: passive-voice 155
Dale: positional-references 5
Dale: undefined-acronyms 4
Dale: wordiness 54
Skipped (needs manual review) Reason

| docs/dataclassification/5.8/contentconfigurationoverview/introduction/addsource/addsingledatabase/databaseconfigwizard.md:42 — Dale: wordiness | 'Contains the list of all tables in the database, followed by alphabetically' is garbled; intended meaning ambiguous |
| docs/dataclassification/5.8/contentconfigurationoverview/introduction/addsource/addsingledatabase/databaseconfigwizard.md:47 — Dale: negative-assumptions | 'Include? — Select if you want to disable crawling for this table' contradicts the column name; correct behavior unclear |
| docs/dataclassification/5.8/contentconfigurationoverview/introduction/addsource/exchangeserverews.md:10 — Dale: wordiness | stray opening quotation mark before 'Microsoft is deprecating EWS' is a punctuation typo outside Dale rule scope |
| docs/dataclassification/5.8/contentconfigurationoverview/introduction/manage/introduction/sposubsiteandlistprocessing.md:23 — Dale: undefined-acronyms | 'QS' is product-specific but its expansion isn't verifiable from surrounding docs |
| docs/dataclassification/5.8/contentconfigurationoverview/introduction/manage/manage.md:22 — Dale: wordiness | 'select to Re-Index an Item and child of a child of so forth child element' is garbled; intended meaning ambiguous |
| docs/dataclassification/5.8/contentconfigurationoverview/introduction/manage/manage.md:56 — Dale: positional-references | 'the corresponding button above' refers to physical UI placement, not to other document content |
| docs/dataclassification/5.8/contentconfigurationoverview/taxonomies/clues/cluestypes.md:31 — Dale: wordiness | 'see Types of Cluessection for full REGEX support' — unclear which section the cross-reference targets |
| docs/dataclassification/5.8/contentconfigurationoverview/workflows/overview.md:22 — Dale: exclamatory-sentences | rhetorical marketing question ('Looking for real-life use cases...?') — rewriting would change authorial intent about linking to training materials |
| docs/dataclassification/5.8/contentconfigurationoverview/workflows/runlog.md:17 — Dale: passive-voice | heading text — not modified to avoid breaking anchor links |
| docs/dataclassification/5.8/dataanalysisoverview/dsar/overview.md:13 — Dale: minimizing-difficulty | 'simple, intuitive interface' is marketing phrasing about the UI rather than a claim about task difficulty; removal would change the author's product framing |
| docs/dataclassification/5.8/dataanalysisoverview/dsar/viewsearchresults.md:27 — Dale: wordiness | 'when the search was run, who, and when reviewed search request results and log' is garbled; intended meaning ambiguous |
| docs/dataclassification/5.8/dataanalysisoverview/reportingintroduction/manage.md:83 — Dale: wordiness | 'described in the user story' internal reference — unclear what it should point to |
| docs/dataclassification/5.8/index.md:32 — Dale: passive-voice | heading text — not modified to avoid breaking anchor links |
| docs/dataclassification/5.8/introduction/introduction/exchange/azureappexchangeonlinemfa.md:60 — Dale: wordiness | stray '8. . Copy your application ID' punctuation typo is outside Dale rule scope |
| docs/dataclassification/5.8/introduction/introduction/spomodernauth.md:105 — Dale: wordiness | 'For taxonomy manager to full operate' typo is outside Dale rule scope |
| docs/dataclassification/5.8/introduction/requirements/softwarerequirements.md:12 — Dale: positional-references | 'Windows 2012 R2 and above' refers to version ordering, not document position |
| docs/dataclassification/5.8/systemconfigurationoverview/administration/indexmaintenance/indexmaintenance.md:7 — Dale: passive-voice | heading text — not modified to avoid breaking anchor links |
| docs/dataclassification/5.8/systemconfigurationoverview/configuration/configmetadata.md:50 — Dale: wordiness | 'you may need to specify what is the role you stand for these documents' is garbled; intended meaning ambiguous |
| docs/dataclassification/5.8/systemconfigurationoverview/configuration/coreconfiguration/details.md:10 — Dale: wordiness | 'The SQL database provides the list of the SQL database' appears duplicated/truncated; intended meaning unclear |

Ask @claude on this PR if you'd like an explanation of any fix.

@jth-nw
jth-nw deployed to development September 30, 2026 19:00 — with GitHub Actions Active
@github-actions

github-actions Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Code Review

No correctness issues found. Scope note: this PR is 300+ files, but only two are non-content — src/config/products.js and sidebars/dataclassification/5.8.js. Everything else is markdown under docs/dataclassification/5.8/ and .webp assets under static/images/dataclassification/5.8/. No workflow, script, or docusaurus.config.js changes.

src/config/products.js — correct

The version bump is internally consistent: 5.8 is prepended with isLatest: true, 5.7 is flipped to isLatest: false, and defaultVersion is moved to 5.8 in the same change. Both signals matter and both were updated — a partial edit here is the usual failure mode and it was avoided.

I checked for hardcoded dataclassification version references elsewhere that would need a matching update. There are none: the client-redirect block in docusaurus.config.js derives everything from getDefaultVersion()/getActiveVersions(), and scripts/copy-kb-to-versions.mjs imports getActiveVersions from products.js, so 5.8 is picked up by the KB pipeline automatically. src/config/redirects/ has no DC entries.

The createRedirects hook remaps unversioned /docs/dataclassification/<page> aliases from 5.7 to 5.8. That would silently 404 any page present in 5.7 but absent in 5.8 — the 5.8 and 5.7 markdown file trees are identical (206 files each, same relative paths), so no alias breaks.

sidebars/dataclassification/5.8.js — correct

Pure type: autogenerated with dirName: ".", byte-identical to 5.7.js. No hand-written doc IDs, so no broken-sidebar-ref risk. The 44 _category_.json files are also byte-identical in aggregate to the 44 in 5.7, so sidebar ordering and category links carry over unchanged.

Build-breaking checks (all clean)

onBrokenMarkdownLinks and onBrokenAnchors are both throw, so these are hard failures rather than warnings:

  • Internal links — 353 .md links, all absolute /docs/dataclassification/5.8/..., matching the 5.7 convention. Every target resolves to a file on disk. Zero links leak to 5.7 or 5.6.2.
  • Anchors — all 4 distinct anchor targets resolve to real headings (#enabling-dqs-mode-5710-and-later-versions, #step-1-create-the-app, #step-5-obtain-tenant-id, #step-2-create-and-register-a-new-app-in-azure-ad).
  • Images — 277 references, all under /images/dataclassification/5.8/; all 234 distinct paths exist on disk. No refs point at 5.7 assets.
  • MDX hazards — raw angle-bracket tag counts are identical to 5.7 (<li>, <ul>, <servername>, etc.), so no new JSX-parsing hazards were introduced.
  • Frontmatter — every file has frontmatter. index.md has no title:, but neither does 5.7/index.md; it uses sidebar_label plus an H1, matching the existing pattern.

Security

No credentials, API keys, or tokens in the new content. The two GUID matches are Microsoft support-article IDs inside URLs. CodeQL passed.

Minor (non-blocking)

Three added images are never referenced by any 5.8 page:

  • static/images/dataclassification/5.8/admin/sources/exchangemailbox/exchangeonline_cfg_modern_auth_thumb_0_0.webp
  • static/images/dataclassification/5.8/admin/sources/sourcegroups/addtogroup.webp
  • static/images/dataclassification/5.8/admin/sources/sourcegroups/dynamicsourcegroups/dynamicsourcegroupex.webp

These are orphaned in 5.7 as well, so this is a carried-over pattern rather than a regression — just dead weight in the repo.

build-and-deploy was still pending when I finished, so the above is static analysis rather than a confirmed build. Content and style are handled by the separate doc-review workflow and were not assessed here.

This branch was successfully deployed

1 active deployment
development — 0f8273c2 Deployed Sep 30, 2026 by jth-nw via build-and-deploy #4273
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@graham-simms-nwx graham-simms-nwx graham-simms-nwx left review comments
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@BenHayman-Netwrix @graham-simms-nwx @DanPiazza-Netwrix @jth-nw