elharo commented on code in PR #42: URL: https://github.com/apache/maven-changes-plugin/pull/42#discussion_r1855450105
########## src/main/java/org/apache/maven/plugins/changes/model/AbstractAction.java: ########## @@ -0,0 +1,95 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Abstract calls with helper methods for {@link Action} + */ +public abstract class AbstractAction { + + private List<DueTo> dueTosList; + + private List<String> fixedIssueList; + + public abstract String getDueTo(); + + public abstract String getDueToEmail(); + + public abstract String getFixedIssuesString(); + + /** + * Parse due-to and due-to-email attributes Review Comment: period ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractAction.java: ########## @@ -0,0 +1,95 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Abstract calls with helper methods for {@link Action} + */ +public abstract class AbstractAction { + + private List<DueTo> dueTosList; + + private List<String> fixedIssueList; + + public abstract String getDueTo(); + + public abstract String getDueToEmail(); + + public abstract String getFixedIssuesString(); + + /** + * Parse due-to and due-to-email attributes + * + * @return a List of due-to person + */ + public List<DueTo> getDueTos() { + if (dueTosList != null) { + return dueTosList; + } + dueTosList = new ArrayList<>(); Review Comment: This doesn't feel thread-safe. Not sure how much that matters. It's hard to analyze because of the split between the superclass and the concrete subclass. See https://stackoverflow.com/questions/8297705/how-to-implement-thread-safe-lazy-initialization ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractAction.java: ########## @@ -0,0 +1,95 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Abstract calls with helper methods for {@link Action} + */ +public abstract class AbstractAction { + + private List<DueTo> dueTosList; + + private List<String> fixedIssueList; + + public abstract String getDueTo(); + + public abstract String getDueToEmail(); + + public abstract String getFixedIssuesString(); + + /** + * Parse due-to and due-to-email attributes + * + * @return a List of due-to person + */ + public List<DueTo> getDueTos() { + if (dueTosList != null) { + return dueTosList; + } + dueTosList = new ArrayList<>(); + List<String> dueTos = new ArrayList<>(); + List<String> dueToEmails = new ArrayList<>(); + + if (getDueTo() != null) { + Arrays.stream(getDueTo().split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .forEach(dueTos::add); + } + + if (getDueToEmail() != null) { + Arrays.stream(getDueToEmail().split(",")).map(String::trim).forEach(dueToEmails::add); + } + + while (dueToEmails.size() < dueTos.size()) { + dueToEmails.add(""); + } + + for (int i = 0; i < dueTos.size(); i++) { + DueTo dueTo = new DueTo(); + dueTo.setName(dueTos.get(i)); + dueTo.setEmail(dueToEmails.get(i)); + dueTosList.add(dueTo); + } + return dueTosList; + } + + /** + * Parse getFixedIssues attribute. + * + * @return a fixed issues as list Review Comment: a list of fixed issues ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractAction.java: ########## @@ -0,0 +1,95 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Abstract calls with helper methods for {@link Action} + */ +public abstract class AbstractAction { + + private List<DueTo> dueTosList; + + private List<String> fixedIssueList; + + public abstract String getDueTo(); + + public abstract String getDueToEmail(); + + public abstract String getFixedIssuesString(); + + /** + * Parse due-to and due-to-email attributes + * + * @return a List of due-to person + */ + public List<DueTo> getDueTos() { + if (dueTosList != null) { + return dueTosList; + } + dueTosList = new ArrayList<>(); + List<String> dueTos = new ArrayList<>(); + List<String> dueToEmails = new ArrayList<>(); + + if (getDueTo() != null) { + Arrays.stream(getDueTo().split(",")) + .map(String::trim) + .filter(s -> !s.isEmpty()) + .forEach(dueTos::add); + } + + if (getDueToEmail() != null) { + Arrays.stream(getDueToEmail().split(",")).map(String::trim).forEach(dueToEmails::add); + } + + while (dueToEmails.size() < dueTos.size()) { + dueToEmails.add(""); + } + + for (int i = 0; i < dueTos.size(); i++) { + DueTo dueTo = new DueTo(); + dueTo.setName(dueTos.get(i)); + dueTo.setEmail(dueToEmails.get(i)); + dueTosList.add(dueTo); + } + return dueTosList; + } + + /** + * Parse getFixedIssues attribute. + * + * @return a fixed issues as list + */ + public List<String> getFixedIssues() { + if (fixedIssueList != null) { + return fixedIssueList; + } + fixedIssueList = new ArrayList<>(); Review Comment: I'd use a local variable here and set the list before returning. ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractRelease.java: ########## @@ -0,0 +1,58 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Abstract calls with helper methods for {@link Release} + */ +public abstract class AbstractRelease { + + private final List<Component> components = new ArrayList<>(); + + public abstract List<Action> getActions(); + + /** + * Retrieve action list by given type Review Comment: period ########## src/main/java/org/apache/maven/plugins/changes/ChangesReportGenerator.java: ########## @@ -226,8 +224,8 @@ private void constructAction(Sink sink, ResourceBundle bundle, Action action) { sink.text("."); } - if (StringUtils.isNotEmpty(action.getDueTo()) || (!action.getDueTos().isEmpty())) { - constructDueTo(sink, action, bundle, action.getDueTos()); + if (!action.getDueTos().isEmpty()) { Review Comment: I guess dueTos can't be null here? ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractAction.java: ########## @@ -0,0 +1,95 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Abstract calls with helper methods for {@link Action} Review Comment: Abstract class period at end ########## src/main/java/org/apache/maven/plugins/changes/model/AbstractRelease.java: ########## @@ -0,0 +1,58 @@ +/* + * 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. + */ +package org.apache.maven.plugins.changes.model; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +/** + * Abstract calls with helper methods for {@link Release} Review Comment: period -- 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-unsubscr...@maven.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
