7d890be7b7
As Ied977dbac shows, the 'unresolved' field is unused for robot comments. It's very likely that it was never meant for use by robot comments and was just unintentionally inherited from human comments. Besides the field not being copied in PostReview.Op [1], which one could argue was just an oversight, the constructor of the class RobotComment passes a hard-coded 'false' for the 'unresolved' field [2]. In addition, we never added behavior for unresolved robot comments to the frontend. They would just appear as other robot comments to users (e.g. have the same color). To not confuse future developers and users of Gerrit's API, we remove this unused functionality. Before anybody complains that there might be a use case for which the 'unresolved' field of robot comments would be helpful: We can always add that field back. However, we should carefully consider whether other concepts aren't better suited than such a boolean field. For instance, a severity parameter for robot comments might make much more sense (e.g. to distinguish between informational-only and more severe robot comments). There's one location where we couldn't completely remove the 'unresolved' parameter: RobotCommentInfo inherits from CommentInfo and we don't have another class which could be used as super class. For robot comments, that field will always be unset, though, and hence it will never appear in the REST API output. If we want to make this clearer in the code, we should clean up the hierarchy in a future change. Moving the field 'unresolved' from Comment to HumanComment doesn't make a difference for the NoteDb storage format. For this reason, this change shouldn't break existing comments. A local, manual test with existing human comments succeeded. [1] https://gerrit.googlesource.com/gerrit/+/6d90ff931199fb3bf3d65b876d3dd9efc27bf2a0/java/com/google/gerrit/server/restapi/change/PostReview.java#1103 [2] https://gerrit.googlesource.com/gerrit/+/6d90ff931199fb3bf3d65b876d3dd9efc27bf2a0/java/com/google/gerrit/entities/RobotComment.java#38 Change-Id: If4e1d76a981f9fedde822238a20c8f18da6d7b25
76 lines
2.2 KiB
Java
76 lines
2.2 KiB
Java
// Copyright (C) 2020 The Android Open Source Project
|
|
//
|
|
// Licensed 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 com.google.gerrit.entities;
|
|
|
|
import java.sql.Timestamp;
|
|
import java.util.Objects;
|
|
|
|
/**
|
|
* This class represents inline human comments in NoteDb. This means it determines the JSON format
|
|
* for inline comments in the revision notes that NoteDb uses to persist inline comments.
|
|
*
|
|
* <p>Changing fields in this class changes the storage format of inline comments in NoteDb and may
|
|
* require a corresponding data migration (adding new optional fields is generally okay).
|
|
*
|
|
* <p>Consider updating {@link #getApproximateSize()} when adding/changing fields.
|
|
*/
|
|
public class HumanComment extends Comment {
|
|
|
|
public boolean unresolved;
|
|
|
|
public HumanComment(
|
|
Key key,
|
|
Account.Id author,
|
|
Timestamp writtenOn,
|
|
short side,
|
|
String message,
|
|
String serverId,
|
|
boolean unresolved) {
|
|
super(key, author, writtenOn, side, message, serverId);
|
|
this.unresolved = unresolved;
|
|
}
|
|
|
|
public HumanComment(HumanComment comment) {
|
|
super(comment);
|
|
}
|
|
|
|
@Override
|
|
public int getApproximateSize() {
|
|
return super.getCommentFieldApproximateSize();
|
|
}
|
|
|
|
@Override
|
|
public String toString() {
|
|
return toStringHelper().add("unresolved", unresolved).toString();
|
|
}
|
|
|
|
@Override
|
|
public boolean equals(Object otherObject) {
|
|
if (!(otherObject instanceof HumanComment)) {
|
|
return false;
|
|
}
|
|
if (!super.equals(otherObject)) {
|
|
return false;
|
|
}
|
|
HumanComment otherComment = (HumanComment) otherObject;
|
|
return unresolved == otherComment.unresolved;
|
|
}
|
|
|
|
@Override
|
|
public int hashCode() {
|
|
return Objects.hash(super.hashCode(), unresolved);
|
|
}
|
|
}
|