Merge "Documentation: clarify how the default branch is set"
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
index b4a5cef..c6d9fb4 100644
--- a/Documentation/config-accounts.txt
+++ b/Documentation/config-accounts.txt
@@ -343,6 +343,12 @@
The `accountId` field is mandatory. The `email` and `password` fields
are optional.
+Note that git will automatically nest these notes at varying levels. If
+refs/meta/external-ids:7c/2a55657d911109dbc930836e7a770fb946e8ef is not
+found then check
+refs/meta/external-ids:7c/2a/55657d911109dbc930836e7a770fb946e8ef and
+so on.
+
The external IDs are maintained by Gerrit. This means users are not
allowed to manually edit their external IDs. Only users with the
link:access-control.html#capability_accessDatabase[Access Database]
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index b2e1589..1db27d5 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2729,8 +2729,8 @@
[source, java]
----
import java.util.Optional;
-import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.common.data.SubmitRecord.Status;
+import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRecord.Status;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
diff --git a/java/com/google/gerrit/entities/CoreDownloadSchemes.java b/java/com/google/gerrit/entities/CoreDownloadSchemes.java
index 37c10f1..9bcd365 100644
--- a/java/com/google/gerrit/entities/CoreDownloadSchemes.java
+++ b/java/com/google/gerrit/entities/CoreDownloadSchemes.java
@@ -21,6 +21,7 @@
public static final String HTTP = "http";
public static final String SSH = "ssh";
public static final String REPO_DOWNLOAD = "repo";
+ public static final String REPO = "repo";
private CoreDownloadSchemes() {}
}
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 681d0bd..4cb52b7 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -27,7 +27,6 @@
/** Preferred method to download a change. */
public enum DownloadCommand {
- REPO_DOWNLOAD,
PULL,
CHECKOUT,
CHERRY_PICK,
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 1650421..220e683 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -121,12 +121,18 @@
// Single Timestamp overhead.
private static final int T = O + 8;
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Take all columns and all collection sizes into account, but use estimated average element
+ * sizes rather than iterating over collections. Numbers are largely hand-wavy based on
+ * http://stackoverflow.com/questions/258120/what-is-the-memory-consumption-of-an-object-in-java
+ *
+ * <p>Should be kept up to date with {@link ChangeNotesState}. Please, keep weights listed in
+ * the same order as fields.
+ */
@Override
public int weigh(Key key, ChangeNotesState state) {
- // Take all columns and all collection sizes into account, but use
- // estimated average element sizes rather than iterating over collections.
- // Numbers are largely hand-wavy based on
- // http://stackoverflow.com/questions/258120/what-is-the-memory-consumption-of-an-object-in-java
return P
+ O
+ 20 // metaId
@@ -138,6 +144,7 @@
+ K // owner
+ P
+ str(state.columns().branch())
+ + P // status
+ P
+ patchSetId() // currentPatchSetId
+ P
@@ -148,9 +155,16 @@
+ str(state.columns().originalSubject())
+ P
+ str(state.columns().submissionId())
- + P // status
+ + 1 // isPrivate
+ + 1 // workInProgress
+ + 1 // reviewStarted
+ + P
+ + K // revertOf
+ + P
+ + patchSetId() // cherryPickOf
+ P
+ set(state.hashtags(), str(10))
+ + str(state.serverId()) // serverId
+ P
+ list(state.patchSets(), patchSet())
+ P
@@ -177,9 +191,6 @@
+ list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
- + 1 // isPrivate
- + 1 // workInProgress
- + 1 // reviewStarted
+ I; // updateCount
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index fa32686..27cfb70 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -81,6 +81,9 @@
* <p>One instance is the output of a single {@link ChangeNotesParser}, and contains types required
* to support public methods on {@link ChangeNotes}. It is intended to be cached in-process.
*
+ * <p>When new fields are added to the {@link ChangeNotesState}, {@link
+ * ChangeNotesCache.Weigher#weigh} should be updated.
+ *
* <p>Note that {@link ChangeNotes} contains more than just a single {@code ChangeNoteState}, such
* as per-draft information, so that class is not cached directly.
*/
diff --git a/plugins/download-commands b/plugins/download-commands
index 87e3930..cfa03bc 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 87e3930cea7c06aea454998abdddf6515a9f103b
+Subproject commit cfa03bc5e7a7e1e27b83f2dba60e9a9eb7c8f4aa
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 8989440..51ce9b4 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -114,6 +114,7 @@
UIActionInfo,
} from '../../shared/gr-js-api-interface/gr-change-actions-js-api';
import {fireAlert} from '../../../utils/event-util';
+import {CODE_REVIEW} from '../../../utils/label-util';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -947,6 +948,16 @@
return null;
}
}
+ // Allow the user to use quick approve to vote the max score on code review
+ // even if it is already granted.
+ if (
+ !result &&
+ this.change.labels[CODE_REVIEW] &&
+ this._getLabelStatus(this.change.labels[CODE_REVIEW]) === LabelStatus.OK
+ ) {
+ result = CODE_REVIEW;
+ }
+
if (result) {
const score = this.change.permitted_labels[result].slice(-1)[0];
const labelInfo = this.change.labels[result];
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
index ae49a57..1098760 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
@@ -1717,6 +1717,31 @@
.querySelector('gr-button[data-action-key=\'review\']');
assert.equal(approveButton.getAttribute('data-label'), 'bar+2');
});
+
+ test('added when can approve an already-approved code review label',
+ () => {
+ element.change = {
+ current_revision: 'abc1234',
+ labels: {
+ 'Code-Review': {
+ approved: {},
+ values: {
+ ' 0': '',
+ '+1': '',
+ '+2': '',
+ },
+ },
+ },
+ permitted_labels: {
+ 'Code-Review': [' 0', '+1', '+2'],
+ },
+ };
+ flush();
+ const approveButton =
+ element.shadowRoot
+ .querySelector('gr-button[data-action-key=\'review\']');
+ assert.isNotNull(approveButton);
+ });
});
test('adds download revision action', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 108f9ed..7759b7b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -1558,7 +1558,7 @@
'changeComments, patchRange and diffPrefs must be set'
);
}
- diffElem.comments = this.changeComments.getCommentsBySideForFile(
+ diffElem.threads = this.changeComments.getThreadsBySideForFile(
file,
this.patchRange,
this.projectConfig
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index b01a110..683d887 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -38,7 +38,7 @@
RevisionId,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
-import {CommentSide} from '../../../constants/constants';
+import {CommentSide, Side} from '../../../constants/constants';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {
Comment,
@@ -313,6 +313,30 @@
return allDrafts;
}
+ _addCommentSide(comments: TwoSidesComments) {
+ const allComments = [];
+ for (const side of [Side.LEFT, Side.RIGHT]) {
+ // This is needed by the threading.
+ for (const comment of comments[side]) {
+ comment.__commentSide = side;
+ }
+ allComments.push(...comments[side]);
+ }
+ return allComments;
+ }
+
+ getThreadsBySideForPath(
+ path: string,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForPath(path, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a path and
* patch-range. Returns an object with left and right properties mapping to
@@ -371,6 +395,18 @@
};
}
+ getThreadsBySideForFile(
+ file: PatchSetFile,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForFile(file, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a file and
* patch-range. Returns an object with left and right properties mapping to
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
index 1ae302a..ecedb28 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
@@ -105,6 +105,8 @@
row.classList.add('diff-row', 'side-by-side');
row.setAttribute('left-type', leftLine.type);
row.setAttribute('right-type', rightLine.type);
+ // TabIndex makes screen reader read a row when navigating with j/k
+ row.tabIndex = -1;
row.appendChild(this._createBlameCell(leftLine.beforeNumber));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
index 04ac472..2011a59 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -104,6 +104,8 @@
_createRow(line: GrDiffLine) {
const row = this._createElement('tr', line.type);
row.classList.add('diff-row', 'unified');
+ // TabIndex makes screen reader read a row when navigating with j/k
+ row.tabIndex = -1;
row.appendChild(this._createBlameCell(line.beforeNumber));
let lineNumberEl = this._createLineEl(
line,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index aa769c6..4b4f429 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -31,8 +31,7 @@
isMergeParent,
isNumber,
} from '../../../utils/patch-set-util';
-import {CommentThread, createCommentThreads} from '../../../utils/comment-util';
-import {TwoSidesComments} from '../gr-comment-api/gr-comment-api';
+import {CommentThread} from '../../../utils/comment-util';
import {customElement, observe, property} from '@polymer/decorators';
import {
CommitRange,
@@ -192,8 +191,8 @@
@property({type: Boolean})
noRenderOnPrefsChange = false;
- @property({type: Object, observer: '_commentsChanged'})
- comments?: TwoSidesComments;
+ @property({type: Object, observer: '_threadsChanged'})
+ threads?: CommentThread[];
@property({type: Boolean})
lineWrapping = false;
@@ -674,21 +673,12 @@
return isImageDiff(diff);
}
- _commentsChanged(newComments: TwoSidesComments) {
- const allComments = [];
- for (const side of [Side.LEFT, Side.RIGHT]) {
- // This is needed by the threading.
- for (const comment of newComments[side]) {
- comment.__commentSide = side;
- }
- allComments.push(...newComments[side]);
- }
+ _threadsChanged(threads: CommentThread[]) {
// Currently, the only way this is ever changed here is when the initial
- // comments are loaded, so it's okay performance wise to clear the threads
+ // threads are loaded, so it's okay performance wise to clear the threads
// and recreate them. If this changes in future, we might want to reuse
// some DOM nodes here.
this._clearThreads();
- const threads = createCommentThreads(allComments);
for (const thread of threads) {
const threadEl = this._createThreadElement(thread);
this._attachThreadElement(threadEl);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index c169bf4..bb0ed61 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -65,11 +65,7 @@
GrDropdownList,
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {
- ChangeComments,
- GrCommentApi,
- TwoSidesComments,
-} from '../gr-comment-api/gr-comment-api';
+import {ChangeComments, GrCommentApi} from '../gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../gr-diff-mode-selector/gr-diff-mode-selector';
import {
ChangeInfo,
@@ -240,9 +236,6 @@
@property({type: Object})
_commentMap?: CommentMap;
- @property({type: Object})
- _commentsForDiff?: TwoSidesComments;
-
@property({
type: Object,
computed: '_computeCommentSkips(_commentMap, _fileList, _path)',
@@ -1013,12 +1006,6 @@
}
this._commentMap = this._getPaths(this._patchRange);
-
- this._commentsForDiff = this._getCommentsForPath(
- this._path,
- this._patchRange,
- this._projectConfig
- );
}
_isFileUnchanged(diff: DiffInfo) {
@@ -1087,7 +1074,13 @@
this._loading = false;
this._initPatchRange();
this._initCommitRange();
- this.$.diffHost.comments = this._commentsForDiff;
+ if (this._changeComments && this._path && this._patchRange) {
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForPath(
+ this._path,
+ this._patchRange,
+ this._projectConfig
+ );
+ }
portedCommentsPromise.then(() => {
this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
});
@@ -1575,13 +1568,11 @@
const file = files[path];
if (file && file.old_path) {
- this._commentsForDiff = this._changeComments.getCommentsBySideForFile(
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
{path, basePath: file.old_path},
patchRange,
projectConfig
);
-
- this.$.diffHost.comments = this._commentsForDiff;
}
}
@@ -1590,22 +1581,6 @@
return this._changeComments.getPaths(patchRange);
}
- _getCommentsForPath(
- path?: string,
- patchRange?: PatchRange,
- projectConfig?: ConfigInfo
- ) {
- if (!path) return undefined;
- if (!patchRange) return undefined;
- if (!this._changeComments) return undefined;
-
- return this._changeComments.getCommentsBySideForPath(
- path,
- patchRange,
- projectConfig
- );
- }
-
_getDiffDrafts() {
if (!this._changeNum) throw new Error('Missing this._changeNum');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 7ef75ed..8e1e5c1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -151,8 +151,10 @@
computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
- getCommentsBySideForPath: () => {},
+ getThreadsBySideForPath: () => {},
+ getThreadsBySideForFile: () => {},
findCommentById: _testOnly_findCommentById,
+
}));
await element._loadComments();
await flush();
@@ -200,11 +202,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
- sinon.stub(element, '_getCommentsForPath').returns({
- left: [{id: 'c1', __commentSide: 'left', line: 10}],
- right: [{id: 'c2', __commentSide: 'right', line: 11}],
- });
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -263,7 +260,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -293,7 +289,6 @@
commentLink: true,
commentId: 'c3',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -1455,7 +1450,6 @@
await flush();
});
test('empty', () => {
- sinon.stub(element, '_getCommentsForPath');
sinon.stub(element, '_getPaths').returns(new Map());
element._initPatchRange();
assert.equal(Object.keys(element._commentMap).length, 0);
@@ -1467,7 +1461,6 @@
'path/to/file/one.cpp': [{patch_set: 3, message: 'lorem'}],
'path-to/file/two.py': [{patch_set: 5, message: 'ipsum'}],
});
- sinon.stub(element, '_getCommentsForPath').returns({meta: {}});
element._changeNum = '42';
element._patchRange = {
basePatchNum: 3,
