Bump #30
Bump #30
Conversation
There was a problem hiding this comment.
Pull request overview
This is a major refactoring/modernization PR that updates the DocBox documentation generator with significant improvements to the HTML output strategy, including Bootstrap 5 migration, enhanced UI/UX features, comprehensive documentation additions, and test infrastructure updates.
Key Changes
- UI Modernization: Complete migration to Bootstrap 5.3.2 with modern design system including dark mode support, improved navigation, and enhanced search functionality
- Strategy Documentation: Extensive JavaDoc-style documentation added to XMI and JSON strategies explaining architecture, usage patterns, and data structures
- Test Infrastructure: Multiple test files updated with runner improvements, coverage configuration, and test case modifications
- Template Enhancements: New features including method search, tabbed interfaces, visibility badges, and improved accessibility
Reviewed changes
Copilot reviewed 79 out of 131 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/specs/XMIStrategyTest.cfc | Tests completely disabled with early return |
| tests/specs/JSONAPIStrategyTest.cfc | Removed directory validation test |
| tests/specs/HTMLAPIStrategyTest.cfc | Removed custom tag test, modified assertion text |
| tests/specs/DocBoxTest.cfc | Removed strategy injection test, updated exception names |
| tests/specs/CommandBoxStrategyTest.cfc | Added debug statement, removed validation test |
| tests/runner.cfm | Enhanced with coverage settings and new parameters |
| tests/run.cfm | Added theme support and JSON strategy generation |
| tests/index.cfm | Complete rewrite with modern structure and BoxLang support |
| tests/coldbox.cfm | New file for ColdBox documentation generation |
| tests/Application.cfc | Added ColdBox resource mapping |
| tests/resources/MockStrategy.cfc | New mock strategy for testing |
| strategy/uml2tools/XMIStrategy.cfc | Extensive documentation and script modernization |
| strategy/json/JSONAPIStrategy.cfc | Comprehensive documentation and BoxLang compatibility |
| strategy/api/themes/frames/resources/templates/* | Multiple new/updated templates with Bootstrap 5 |
| strategy/api/themes/frames/resources/static/* | New CSS, JavaScript libraries, and theme assets |
| // Todo activate later | ||
| return; |
There was a problem hiding this comment.
The XMI strategy tests have been completely disabled with a return statement and a TODO comment. This removes test coverage for the XMI strategy functionality. If these tests are being temporarily disabled due to known issues, consider using xit() or xdescribe() instead of an early return, which provides better visibility in test reports.
|
|
||
| var fileContents = fileRead( testFile ); | ||
|
|
||
| debug( fileContents ) |
There was a problem hiding this comment.
A debug statement has been left in the test code. This should be removed before merging as it will clutter test output in production.
|
|
||
| var fileContents = fileRead( testFile ); | ||
|
|
||
| debug( fileContents ) |
There was a problem hiding this comment.
Missing semicolon after the debug function call. According to JavaScript coding standards in the guidelines, all statements should be properly terminated with semicolons.
| var allClassesHTML = fileRead( allClassesFile ); | ||
| expect( allClassesHTML ).toInclude( | ||
| "HTMLAPIStrategyTest", | ||
| "frameless SPA design", |
There was a problem hiding this comment.
The test assertion text has been changed from "HTMLAPIStrategyTest" to "frameless SPA design". This appears to be testing for the wrong string value. The test should verify that HTMLAPIStrategyTest.cfc is documented in the list of classes, not check for UI design terminology.
| var modifiers = 'public private remote package abstract final static required'; | ||
|
|
||
| // Template syntax components (bx: prefixed) | ||
| var templateComponents = 'argument function set return if else elseif try catch finally import while break continue include property rethrow throw switch case defaultcase output query'; |
There was a problem hiding this comment.
Unused variable templateComponents.
Description
Please include a summary of the changes and which issue(s) is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Please note that all PRs must have tests attached to them
IMPORTANT: Please review the CONTRIBUTING.md file for detailed contributing guidelines.
Issues
All PRs must have an accompanied issue. Please make sure you created it and linked it here.
Type of change
Please delete options that are not relevant.
Checklist