Repository navigation
Conversation
shossain786
left a comment
There was a problem hiding this comment.
Thank you @matz2009 for taking #2. The idea and the tests for the missing-pom case are good, and the branch builds and passes. I need several changes, though.
1. Overlap with #12.
#12 also adds a Detected technologies section and changes the Report record, and the two conflict. I plan to merge #12 first. After that, please rebase onto it and keep your work as the recognised technologies summary (Selenium version, TestNG, JUnit 4/5, WebDriverManager, ExtentReports/Allure) built from #12's dependency list, so the two don't fight over the same section.
2. XML external entities are not disabled.
DocumentBuilderFactory.newInstance() is used as-is, and this tool is meant to analyze projects it has not verified. Please disable DOCTYPE declarations and external entities. #12's BuildFileAnalyzer shows the settings we want.
3. Detection is too broad.
getElementsByTagName("dependency") also matches entries in dependencyManagement and inside plugin blocks. A TestNG dependency in each of three modules also adds , TestNG three times. Please read only direct project/dependencies children and de-duplicate.
4. Unrelated changes.
The diff deletes the rule comments and the class Javadoc and reformats scan(...). Please revert those so the diff shows only the feature. Also change Report in a backward-compatible way (keep the four-argument constructor) so existing callers keep working.
5. Silent failure.
catch (Exception e) { return "Maven"; } hides a broken pom. Please say so in the output, for example Maven (could not parse pom.xml).
Happy to help if any of this is unclear.
|
@shossain786 this pr is now outdated as it's updated code is not already merged you may close this please |
|
@matz2009 heads up — #12 (gradle-support) is already merged, so this branch now has conflicts and needs a rebase. The review above still applies: please rebase onto main, then address points 1–5 (overlap with #12's tech-detection section, XML external entity handling, over-broad dependency matching, unrelated formatting/Javadoc changes, and the silent catch block). If you're not planning to continue this, let us know and we'll close it — otherwise happy to help once it's rebased. |
What does this PR do?
Adds Maven technology detection to the analysis report.
Changes
pom.xmland reports Maven.not detectedwhenpom.xmlis missing.pom.xml.Related issue
Closes #2