Skip to content

Conversation

@hilram7
Copy link
Collaborator

@hilram7 hilram7 commented Jan 15, 2026

Summary

Updates the .NET Dependencies KB for Netwrix Recovery for Active Directory:

  • Relocates KB from staging to correct product directory
  • Updates requirements link to current Recovery for Active Directory docs
  • Replaces Salesforce-hosted image with local copy
  • Removes cross-product KB links (will be restored in a follow-up PR after all 5 products merge)

Changes

File Relocation

  • Moved net_dependencies_for_netwrix_recovery_for_active_directory.md from docs-staging/needs-relocation/ to docs/kb/recoveryad/configuration-and-administration/

Requirements Link

  • Updated link from generic placeholder to current docs: https://docs.netwrix.com/docs/recoveryforactivedirectory/2_6/requirements/server

Image Replacement

  • Added local PowerShell example image to docs/kb/recoveryad/0-images/ka0Qk000000DG8bIAG.png
  • Replaced Salesforce URL with relative path ../0-images/ka0Qk000000DG8bIAG.png
  • Fixed incorrect alt text from "Dialog box for selecting monitoring plan settings" to "PowerShell Example Output"

Cross-Product Links

  • Removed cross-product KB links for broken link prevention
  • Links will be restored in a follow-up PR after all 5 product KBs merge

Technical Updates

Testing

  • ✅ Dev server: KB page loads correctly at /docs/kb/recoveryad/configuration-and-administration/net_dependencies_for_netwrix_recovery_for_active_directory
  • ✅ Image displays properly from local path
  • ✅ Requirements link navigates to correct Recovery for Active Directory docs
  • ✅ KB accessible from sidebar navigation
  • ✅ Build passes without errors

Related

  • Related PRs: Access Analyzer, Activity Monitor, Threat Prevention, Threat Manager
  • Follow-up: Will restore cross-product KB links after all 5 merge

Hilary Ramirez added 4 commits January 13, 2026 14:06
- Moved KB from docs-staging/needs-relocation/ to configuration-and-administration/
- Updated requirements link to recoveryforactivedirectory/2_6/requirements/server
- Fixed cross-product KB links to point to correct product directories
- Removed incorrect .md extensions from internal links
- Updated link text to 'Recovery for Active Directory Requirements'
- Changed URL to point to requirements server page
- Simplified wording to match new docs site structure
…and fix alt text

- Add local PowerShell example image to 0-images folder
- Replace incorrect Salesforce image with PowerShell output image
- Fix alt text from "Dialog box for selecting monitoring plan settings" to "PowerShell Example Output"
- Remove cross-product KB links per Option B strategy
@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@github-actions
Copy link
Contributor

Code Review - PR #49

Summary

This PR successfully relocates and updates the .NET Dependencies KB article for Recovery for Active Directory. The changes are generally well-executed with good attention to detail in updating links and replacing external images with local copies.

✅ Positive Aspects

  1. Good File Organization: The KB article has been correctly moved from staging to its proper product directory
  2. External Dependency Removal: Replacing the Salesforce-hosted image with a local copy improves reliability and reduces external dependencies
  3. Alt Text Correction: Fixed the incorrect alt text from "Dialog box for selecting monitoring plan settings" to "PowerShell Example Output" - much more accurate
  4. Updated Documentation Links: The requirements link now points to the correct specific page rather than a generic landing page
  5. Clean Commit History: Well-structured commits with clear messages describing each change
  6. Testing Coverage: Good testing checklist in the PR description showing the changes were verified

⚠️ Issues Found

1. Inconsistent Product Identifier (Medium Priority)

File: docs/kb/recoveryad/configuration-and-administration/net_dependencies_for_netwrix_recovery_for_active_directory.md:9

The frontmatter uses recoveryad as the product identifier:

products:
  - recoveryad

However, other KB files in the same directory use recovery-active-directory:

  • how-to-reset-multi-factor-authentication.md uses recovery-active-directory
  • recovery_for_ad_web_console_only_available_via_http.md uses recovery-active-directory

Recommendation: For consistency, update line 9 to use recovery-active-directory to match the other KB articles in this product directory.

2. Missing Newline at End of File (Low Priority)

File: docs/kb/recoveryad/configuration-and-administration/net_dependencies_for_netwrix_recovery_for_active_directory.md:43

The file ends without a trailing newline after the last link. While not critical, it's a common convention to include a final newline in text files.

3. Image Format Consideration (Low Priority)

File: docs/kb/recoveryad/0-images/ka0Qk000000DG8bIAG.png

The image is 53KB in PNG format (783x169). According to the README.md guidelines:

Format: Use .webp for performance

Recommendation: Consider converting the image to WebP format for better performance, especially since the README explicitly recommends WebP for all documentation images.

🔍 Code Quality Assessment

  • Markdown Structure: ✅ Well-formatted
  • Link Validity: ✅ Links appear correct
  • PowerShell Code Block: ✅ Properly formatted with syntax highlighting
  • Frontmatter: ⚠️ Mostly correct but has product identifier inconsistency
  • Image References: ✅ Correct relative path usage

🔒 Security Considerations

  • ✅ No security concerns identified
  • ✅ Replacing external Salesforce link with local image is a security improvement (reduces external dependencies and potential link rot)

⚡ Performance Considerations

  • ⚠️ PNG image could be optimized to WebP format for better performance
  • ✅ Local image hosting is better for performance than external Salesforce links

🧪 Test Coverage

  • ✅ PR description shows comprehensive manual testing
  • ✅ Build is passing (in progress but no failures reported)
  • ✅ Changes are limited in scope and low-risk

📝 Documentation Quality

  • ✅ The KB article content is clear and well-structured
  • ✅ PowerShell command is accurate and properly documented
  • ✅ The note about .NET Framework vs ASP.NET Core is helpful

Recommendations

  1. Fix the product identifier inconsistency (change recoveryad to recovery-active-directory)
  2. Add trailing newline at end of file
  3. Consider converting image to WebP format per repository guidelines
  4. The intentional removal of cross-product KB links is well-documented and appropriate

Overall Assessment

APPROVE

This is a solid PR that improves the documentation structure and reliability. The issues identified are minor and don't block the PR from being merged. The most important fix would be the product identifier consistency, which can be addressed in a quick follow-up commit or in the next PR.

Great work on the detailed PR description and testing checklist!

@hilram7 hilram7 merged commit f9fbd99 into dev Jan 17, 2026
7 checks passed
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.

3 participants