-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Build Plan API to accept file ownership in layers #2494
Conversation
@@ -121,12 +158,13 @@ public boolean equals(Object other) { | |||
FileEntry otherFileEntry = (FileEntry) other; | |||
return sourceFile.equals(otherFileEntry.sourceFile) | |||
&& extractionPath.equals(otherFileEntry.extractionPath) | |||
&& Objects.equals(permissions, otherFileEntry.permissions) | |||
&& Objects.equals(modificationTime, otherFileEntry.modificationTime); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps Objects.equals
was necessary in the past when permissions
and modificationTime
were @Nullable
. The java.util.Objects.equals
is implemented like the following, so we don't need it.
public static boolean equals(Object a, Object b) {
return (a == b) || (a != null && a.equals(b));
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine, just have a minor comment.
@@ -265,6 +326,10 @@ public FileEntriesLayer build() { | |||
DEFAULT_MODIFICATION_TIME_PROVIDER = | |||
(sourcePath, destinationPath) -> DEFAULT_MODIFICATION_TIME; | |||
|
|||
/** Provider that returns default file ownership ("0:0"). */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment and the return value don't really line up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. Also updated the Build Plan Spec doc with the last commit.
Merging this now to start releasing early. |
Towards writing an extension for #1257 and #1955, as well as a workaround for #1270.
Once merged, we need to re-publish all the libraries one after another.