You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2021/09/29 23:46:49 UTC
[GitHub] [maven] khmarbaise opened a new pull request #558: [MNG-7272] - Code Improvement - II
khmarbaise opened a new pull request #558:
URL: https://github.com/apache/maven/pull/558
Following this checklist to help us incorporate your
contribution quickly and easily:
- [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [ ] Each commit in the pull request should have a meaningful subject line and body.
- [ ] Format the pull request title like `[MNG-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MNG-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [ ] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
- [x] You have run the [Core IT][core-its] successfully.
If your pull request is about ~20 lines of code you don't need to sign an
[Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [x] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
[core-its]: https://maven.apache.org/core-its/core-it-suite/
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] pzygielo commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719132710
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -43,16 +43,16 @@
* @author Benjamin Bentmann
*/
public class DefaultProjectDependencyGraph
- implements ProjectDependencyGraph
+ implements ProjectDependencyGraph
{
- private final ProjectSorter sorter;
+ private ProjectSorter sorter;
- private final List<MavenProject> allProjects;
+ private List<MavenProject> allProjects;
- private final Map<MavenProject, Integer> order;
+ private Map<MavenProject, Integer> order;
- private final Map<String, MavenProject> projects;
Review comment:
Why?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] pzygielo commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719133718
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -62,38 +62,30 @@
* @throws CycleDetectedException
*/
public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
{
- super();
- this.allProjects = Collections.unmodifiableList( new ArrayList<>( projects ) );
- this.sorter = new ProjectSorter( projects );
- this.order = new HashMap<>();
- this.projects = new HashMap<>();
- List<MavenProject> sorted = this.sorter.getSortedProjects();
- for ( int index = 0; index < sorted.size(); index++ )
- {
- MavenProject project = sorted.get( index );
- String id = ProjectSorter.getId( project );
- this.projects.put( id, project );
- this.order.put( project, index );
- }
+ init( projects, projects );
}
/**
* Creates a new project dependency graph based on the specified projects.
*
* @param allProjects All collected projects.
- * @param projects The projects to create the dependency graph with.
- *
+ * @param projects The projects to create the dependency graph with.
* @throws DuplicateProjectException
* @throws CycleDetectedException
* @since 3.5.0
*/
public DefaultProjectDependencyGraph( final List<MavenProject> allProjects,
final Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
+ {
+ init( allProjects, projects );
+ }
+
+ private void init( Collection<MavenProject> allProjects, final Collection<MavenProject> projects )
Review comment:
Why not remove this new method, and in `DefaultProjectDependencyGraph( Collection<MavenProject> )` use this Ctor, and bring final fields back?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719159185
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -62,38 +62,30 @@
* @throws CycleDetectedException
*/
public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
{
- super();
- this.allProjects = Collections.unmodifiableList( new ArrayList<>( projects ) );
- this.sorter = new ProjectSorter( projects );
- this.order = new HashMap<>();
- this.projects = new HashMap<>();
- List<MavenProject> sorted = this.sorter.getSortedProjects();
- for ( int index = 0; index < sorted.size(); index++ )
- {
- MavenProject project = sorted.get( index );
- String id = ProjectSorter.getId( project );
- this.projects.put( id, project );
- this.order.put( project, index );
- }
+ init( projects, projects );
}
/**
* Creates a new project dependency graph based on the specified projects.
*
* @param allProjects All collected projects.
- * @param projects The projects to create the dependency graph with.
- *
+ * @param projects The projects to create the dependency graph with.
* @throws DuplicateProjectException
* @throws CycleDetectedException
* @since 3.5.0
*/
public DefaultProjectDependencyGraph( final List<MavenProject> allProjects,
final Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
+ {
+ init( allProjects, projects );
+ }
+
+ private void init( Collection<MavenProject> allProjects, final Collection<MavenProject> projects )
Review comment:
I feel the same. The init isn't that complex that it would require a separate method. Final is a nice thing.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rfscholte commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719298137
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
IMO Maven uses List too often where Collection should have been used. I'm pretty sure this needs to be reverted.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719306831
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
Ok so let us go that direction:
```java
public DefaultProjectDependencyGraph( Collection<MavenProject> allProjects,
Collection<MavenProject> projects )
...
...}
public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
throws CycleDetectedException, DuplicateProjectException
{
this( projects, projects );
}
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] olamy commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
olamy commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r720622728
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( final Collection<MavenProject> projects )
Review comment:
> The original code has the same `final` for parameters: https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java#L92 I just want to keep it the way...but I'm the same opinion to remove `final`..
so remove it ;)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719294804
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
Attention: You are changing a public contract here. Is this intended?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on pull request #558:
URL: https://github.com/apache/maven/pull/558#issuecomment-932710006
Squashed...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] gnodet commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
gnodet commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719507925
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( final Collection<MavenProject> projects )
Review comment:
What's the reasoning behind adding `final` to all parameters ? It's really useless afaik, as the compiler infers them when needed (i.e. when those are referenced in a lambda expression or an anonymous class for example) and it has no performance impact.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719160720
##########
File path: maven-core/src/main/java/org/apache/maven/project/ProjectSorter.java
##########
@@ -126,43 +123,29 @@ public ProjectSorter( Collection<MavenProject> projects )
parent.getVersion(), true, false );
}
- List<Plugin> buildPlugins = project.getBuildPlugins();
- if ( buildPlugins != null )
Review comment:
Yes I know that the old code implied that there might be `null` an option but If you take a look into `project.getBuildPlugins();`
```java
public List<Plugin> getBuildPlugins()
{
if ( getModel().getBuild() == null )
{
return Collections.emptyList();
}
return Collections.unmodifiableList( getModel().getBuild().getPlugins() );
}
```
furthermore the part: `getModel().getBuild().getPlugins()`:
```java
public java.util.List<Plugin> getPlugins()
{
if ( this.plugins == null )
{
this.plugins = new java.util.ArrayList<Plugin>();
}
return this.plugins;
} //-- java.util.List<Plugin> getPlugins()
```
So can't be returning `null`...so the check does not make sense anymore.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719314728
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
> `this(new ArrayList(projects), projects);` ?
This would result in duplication of `new ArrayList<>(..)` because the called ctor starts with:
```java
public DefaultProjectDependencyGraph( Collection<MavenProject> allProjects,
Collection<MavenProject> projects )
throws CycleDetectedException, DuplicateProjectException
{
this.allProjects = Collections.unmodifiableList( new ArrayList<>( allProjects ) );
this.sorter = new ProjectSorter( projects );
this.order = new HashMap<>();
...
I think the easiest way is to change both constructors to use Collection which will solve that...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719314728
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
> `this(new ArrayList(projects), projects);` ?
This would result in duplication of `new ArrayList<>(..)` because the called ctor starts with:
```java
public DefaultProjectDependencyGraph( Collection<MavenProject> allProjects,
Collection<MavenProject> projects )
throws CycleDetectedException, DuplicateProjectException
{
this.allProjects = Collections.unmodifiableList( new ArrayList<>( allProjects ) );
this.sorter = new ProjectSorter( projects );
this.order = new HashMap<>();
```
I think the easiest way is to change both constructors to use Collection which will solve that...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] pzygielo commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
pzygielo commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719125588
##########
File path: maven-core/src/main/java/org/apache/maven/project/ProjectSorter.java
##########
@@ -126,43 +123,29 @@ public ProjectSorter( Collection<MavenProject> projects )
parent.getVersion(), true, false );
}
- List<Plugin> buildPlugins = project.getBuildPlugins();
- if ( buildPlugins != null )
Review comment:
The old code suggested that `null` might be returned from `project.getBuildPlugins()`. Is that no longer true?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] rfscholte commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
rfscholte commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719305664
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
`this(new ArrayList(projects), projects);` ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] michael-o commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719304708
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
The cast is dangerous because you don't know that this is a list actually. Why not wrap it in a list, idiotproof?!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719162777
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -62,38 +62,30 @@
* @throws CycleDetectedException
*/
public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
{
- super();
- this.allProjects = Collections.unmodifiableList( new ArrayList<>( projects ) );
- this.sorter = new ProjectSorter( projects );
- this.order = new HashMap<>();
- this.projects = new HashMap<>();
- List<MavenProject> sorted = this.sorter.getSortedProjects();
- for ( int index = 0; index < sorted.size(); index++ )
- {
- MavenProject project = sorted.get( index );
- String id = ProjectSorter.getId( project );
- this.projects.put( id, project );
- this.order.put( project, index );
- }
+ init( projects, projects );
}
/**
* Creates a new project dependency graph based on the specified projects.
*
* @param allProjects All collected projects.
- * @param projects The projects to create the dependency graph with.
- *
+ * @param projects The projects to create the dependency graph with.
* @throws DuplicateProjectException
* @throws CycleDetectedException
* @since 3.5.0
*/
public DefaultProjectDependencyGraph( final List<MavenProject> allProjects,
final Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ throws CycleDetectedException, DuplicateProjectException
+ {
+ init( allProjects, projects );
+ }
+
+ private void init( Collection<MavenProject> allProjects, final Collection<MavenProject> projects )
Review comment:
Yeah I think it was too late...this morning...If I change the signature of one of the Ctors...this can simply being done...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] asfgit merged pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #558:
URL: https://github.com/apache/maven/pull/558
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r719301712
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( List<MavenProject> projects )
Review comment:
If I don't do that I have to cast explicit...
```java
public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
throws CycleDetectedException, DuplicateProjectException
{
this( (List<MavenProject>) projects, projects );
}
```
Furthermore the current calls to ctor are only using List...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven] khmarbaise commented on a change in pull request #558: [MNG-7272] - Code Improvement - II
Posted by GitBox <gi...@apache.org>.
khmarbaise commented on a change in pull request #558:
URL: https://github.com/apache/maven/pull/558#discussion_r720541987
##########
File path: maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java
##########
@@ -61,39 +61,25 @@
* @throws DuplicateProjectException
* @throws CycleDetectedException
*/
- public DefaultProjectDependencyGraph( Collection<MavenProject> projects )
- throws CycleDetectedException, DuplicateProjectException
+ public DefaultProjectDependencyGraph( final Collection<MavenProject> projects )
Review comment:
The original code has the same `final` for parameters:
https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/graph/DefaultProjectDependencyGraph.java#L92
I just want to keep it the way...but I'm the same opinion to remove `final`..
--
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.
To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org