[GitHub] [maven-doxia] abelsromero commented on a change in pull request #35: DOXIA-614: support source reference in doxia parser

[GitBox <gi...@apache.org>]

abelsromero commented on a change in pull request #35:
URL: https://github.com/apache/maven-doxia/pull/35#discussion_r451014972



##########
File path: doxia-core/src/main/java/org/apache/maven/doxia/parser/AbstractXmlParser.java
##########
@@ -100,7 +98,7 @@
     private boolean validate = false;
 
     /** {@inheritDoc} */
-    public void parse( Reader source, Sink sink )
+    public void parse( Reader source, Sink sink, String reference )

Review comment:
       Effectively, it is not. As explained in the JIRA issue the PR aims to be that the default method instead of the other one with 2 arguments. The way it is implemented right now, even when `Parser` interface has two method, the one with thre is not usable by how it's overriden in `AbstractParser` and never invoked in `DefaultDoxia`.
   The final goal is to deprecate the one with two arguments so that any plugin can obtain the metadata of the sourcefile bien processed.
   
   I am available to discuss online if anyone needs to, because I am bit confused by how this review is proceding. I think the JIRA issue, explanation here and the additional diagram provide a clear description of the goals and changes done which are the complex topics. However, I am seeing a focus on minor things. Furthermore, I raised 2 important points in the JIRA issue which had had no answer and which imo are worth discussing, one related to problems with the build and the other with testing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org