[GitHub] [maven-doxia] abelsromero commented on a change in pull request #35: DOXIA-614: support source reference in doxia parser
abelsromero commented on a change in pull request #35:
URL: https://github.com/apache/maven-doxia/pull/35#discussion_r446685351
##
File path: doxia-core/src/main/java/org/apache/maven/doxia/Doxia.java
##
@@ -52,6 +52,22 @@
void parse( Reader source, String parserId, Sink sink )
throws ParserNotFoundException, ParseException;
+/**
+ * Parses the given source model using a parser with given id,
+ * and emits Doxia events into the given sink.
+ *
+ * @param source not null reader that provides the source document.
+ * You could use newReader methods from {@link
org.codehaus.plexus.util.ReaderFactory}.
+ * @param parserId Identifier for the parser to use.
Review comment:
The exceptions in the oracle reference
(https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#@exception)
do capitalize, should I correct exceptions also accordingly?
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org
For additional commands, e-mail: dev-h...@maven.apache.org
[GitHub] [maven-doxia] elharo commented on a change in pull request #35: DOXIA-614: support source reference in doxia parser
elharo commented on a change in pull request #35:
URL: https://github.com/apache/maven-doxia/pull/35#discussion_r446678771
##
File path: doxia-core/src/test/java/org/apache/maven/doxia/DefaultDoxiaTest.java
##
@@ -0,0 +1,54 @@
+package org.apache.maven.doxia;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.doxia.parser.manager.ParserNotFoundException;
+import org.codehaus.plexus.PlexusTestCase;
+import org.junit.Test;
+
+public class DefaultDoxiaTest extends PlexusTestCase
+{
+
+@Test
+public void testCreatesDefaultDoxia()
+{
+final DefaultDoxia defaultDoxia = new DefaultDoxia();
+
+assertNotNull( defaultDoxia );
+}
+
+@Test
+public void testFailsWhenParserIdDoesNotExist() throws Exception
+{
+final String parserId = "a-parser";
+final Doxia doxia = lookup( Doxia.class );
+
+try
+{
+doxia.getParser( parserId );
+}
+catch ( Exception e )
Review comment:
just catch ParserNotFoundException rather than asserting the type
##
File path: doxia-core/src/main/java/org/apache/maven/doxia/Doxia.java
##
@@ -52,6 +52,22 @@
void parse( Reader source, String parserId, Sink sink )
throws ParserNotFoundException, ParseException;
+/**
+ * Parses the given source model using a parser with given id,
+ * and emits Doxia events into the given sink.
+ *
+ * @param source not null reader that provides the source document.
+ * You could use newReader methods from {@link
org.codehaus.plexus.util.ReaderFactory}.
Review comment:
I'd delete this note. I'd prefer not to encourage extra plexus
dependencies
##
File path: doxia-core/src/test/java/org/apache/maven/doxia/DefaultDoxiaTest.java
##
@@ -0,0 +1,54 @@
+package org.apache.maven.doxia;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.doxia.parser.manager.ParserNotFoundException;
+import org.codehaus.plexus.PlexusTestCase;
+import org.junit.Test;
+
+public class DefaultDoxiaTest extends PlexusTestCase
+{
+
+@Test
+public void testCreatesDefaultDoxia()
+{
+final DefaultDoxia defaultDoxia = new DefaultDoxia();
+
+assertNotNull( defaultDoxia );
+}
+
+@Test
+public void testFailsWhenParserIdDoesNotExist() throws Exception
+{
+final String parserId = "a-parser";
+final Doxia doxia = lookup( Doxia.class );
+
+try
+{
+doxia.getParser( parserId );
Review comment:
test should fail here if exception is not thrown
##
File path: doxia-core/src/main/java/org/apache/maven/doxia/Doxia.java
##
@@ -52,6 +52,22 @@
void parse( Reader source, String parserId, Sink sink )
throws ParserNotFoundException, ParseException;
+/**
+ * Parses the given source model using a parser with given id,
+ * and emits Doxia events into the given sink.
+ *
+ * @param source not null reader that provides the source document.
+ * You could use newReader methods from {@link
org.codehaus.plexus.util.ReaderFactory}.
+ * @param parserId Identifier for the parser to use.
+ * @param sink A sink that consumes the Doxia events.
Review comment:
ditto
##
File path: doxia-core/src/main/java/org/apache/maven/doxia/Doxia.java
##
@@ -52,6 +52,22 @@
void
[GitHub] [maven-doxia] abelsromero opened a new pull request #35: DOXIA-614: support source reference in doxia parser
abelsromero opened a new pull request #35: URL: https://github.com/apache/maven-doxia/pull/35 There's still a thing to fix with test begin discussed in https://issues.apache.org/jira/browse/DOXIA-614. Other than that, the changes are all done and the tests pass. I tried to minimize changes so no tests have been modified and all works as-is. To facilitate review, here is a diagram with changes. Green for added elements, red for removed. . Some observations open for discussion: - Used null insteall of empty string to represent there's no reference at all. I found the empty string to be ambiguous. I noted that `AptParser`, `ConfluenceParser` and `TwikiParser` use empty string, but I consider this implementation specific. 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@maven.apache.org For additional commands, e-mail: dev-h...@maven.apache.org
Re: migrating Apache Resource Bundles from svn to Git
Karl helps everyone who has questions He will always answer any questions I ask no matter how trivial Viel Gluck! -- Sent from my Alcatel GO FLIP V On Jun 25, 2020 2:57 AM, Robert Scholte wrote: I don't mind joining as committer. I actually was surprised Karl Heinz is already PMC. Robert On 25-6-2020 08:37:55, Hervé BOUTEMY wrote: yes, Creadur could be a good home both for these bundles and the Apache parent POM, to manage Apache project conventions for Maven separately from Maven code itself And if some of us want to continue work on it (given we produce plugins that are to be updated in Apache parent POM), they could join Creadur https://creadur.apache.org/ Regards, Hervé Le mercredi 24 juin 2020, 21:40:41 CEST Robert Scholte a écrit : > I've been thinking about asking if project Creadur wants this code, since > they also maintain the rat maven plugin. Looks quite related to me. > > Robert > > On 24-6-2020 00:00:09, Hervé BOUTEMY wrote: > the little discussion now gave me an opinion: > if nobody objects, I'll migrate the bundles to one unique Git repository: > - name: maven-apache-resource-bundles.git > - version will be 1.5-SNAPSHOT > - containing 3 > org.apache.apache.resources:apache-(jar|jar-txt|incubator-disclaimer)-resou > rce-bundle for maven-remote-resources-plugin and > apache-source-release-assembly-descriptor for maven-assembly-plugin > > Regards, > > Hervé > > Le mardi 23 juin 2020, 00:11:41 CEST Hervé BOUTEMY a écrit : > > it seems there is a misunderstanding: it's not 4 parent POMs > > it's resource bundles, that are used with maven-assembly-plugin or maven- > > remote-resources-plugin in Apache parent POM [1] > > > > see https://maven.apache.org/apache-resource-bundles/index.html for some > > description > > > > Regards, > > > > Hervé > > > > [1] https://github.com/apache/maven-apache-parent/blob/master/pom.xml > > > > Le lundi 22 juin 2020, 20:31:16 CEST Robert Scholte a écrit : > > > On 22-6-2020 00:25:39, Hervé BOUTEMY wrote: > > > > I agree: independent release cycles means independent repositories. > > > > > > in the past, we had 4 independent Maven parent POMs [1] under one unique > > > svn trunk, with 4 separate release cycles, many votes/hard maintenance > > > (having a new parent POM release for plugins, for example) and hard to > > > use (version of parent for plugins was not the same as shared parent, or > > > pure maven parent). When switching to Git, we switched to one Git > > > repository and one unique release cycle: a vote for a release remains > > > not > > > so easy (because getting votes is not easy), but at least there is only > > > one for every parent POM. Robert Scholte: I think there's a small > > > difference, these poms all inherit from the apache-parent. If there's a > > > new parent, it makes sense to release them all at once. > > > > > > > > > > > > We have the same initial situation with resource bundles: the > > > independence > > > of the release cycles inside the unique trunk are not written in stone, > > > we > > > can simplify and hope that it will ease maintenance in addition to ease > > > migration. Or we can make efforts to split: for plugins or shared > > > components, splitting was the only reasonable option. > > > > > > > I wonder if they should be part of the Maven group, looks more like a > > > > they > > > > are maintained by us by coincidence. Is there some ASF resources group > > > > to > > > > maintain this (or worth introducing)? > > > > > > exactly like the ASF parent POM > > > If you find someone who want to maintain, don't hesitate > > > Robert Scholte: > > > Let me see if I can get them out of the Maven project. > > > I don't mind maintaining these parents, but I'd prefer have a clear > > > separation on which parts are clearly Maven and which actually ASF > > > > > > thanks, > > > Robert > > > > > > > > > > > > Regards, > > > > > > Hervé > > > > > > [1] https://maven.apache.org/pom/index.html > > > > > > Le dimanche 21 juin 2020, 21:36:37 CEST Robert Scholte a écrit : > > > > I agree: independent release cycles means independent repositories. > > > > I wonder if they should be part of the Maven group, looks more like a > > > > they > > > > are maintained by us by coincidence. Is there some ASF resources group > > > > to > > > > maintain this (or worth introducing)? > > > > > > > > thanks, > > > > Robert > > > > > > > > On 21-6-2020 19:14:47, Michael Osipov wrote: > > > > > > > > Am 2020-06-20 um 00:31 schrieb Hervé BOUTEMY: > > > > > one of the last part not yet done, because there is a choice to do: > > > > > - migrate to one multi-module Git repository? > > > > > - or split each bundle into a separate Git repo? > > > > > > > > These two question can be answered when we know whether these modules > > > > are loosely coupled of strongly interrelated like our POM parents. > > > > If the former, separate repos, if the latter one repo. > > > > > > > > > > > >
