Merge "Document another case where no code owners are suggested"
diff --git a/resources/Documentation/config-guide.md b/resources/Documentation/config-guide.md
index ab8b46a..3202450 100644
--- a/resources/Documentation/config-guide.md
+++ b/resources/Documentation/config-guide.md
@@ -106,13 +106,14 @@
Root code owners can differ from branch to branch.
3. Default code owners:
[Default code owners](backend-find-owners.html#defaultCodeOwnerConfiguration)
- are stored in the code owner config file in the `refs/meta/config` branch
- that apply for all branches (unless inheritance is ignored).\
- The same as root code owners these are experienced developers that can
- approve changes to all the code base if needed.\
- However in contrast to root code owners that apply to all branches (including
- newly created branches), and hence can be used if code owners should be kept
- consistent across all branches.\
+ are stored in the code owner config file (e.g. the `OWNERS` file) in the
+ `refs/meta/config` branch and apply for all branches (unless inheritance is
+ ignored).\
+ The same as root code owners, default code owners are experienced developers
+ that can approve changes to all the code base if needed.\
+ However in contrast to root code owners, default code owners apply to all
+ branches (including newly created branches), and hence can be used if code
+ owners should be kept consistent across all branches.\
A small disadvantage is that this code owner definition is not very well
discoverable since it is stored in the `refs/meta/config` branch, but default
code owners are suggested to users the same way as other code owners.
diff --git a/ui/code-owners-service.js b/ui/code-owners-service.js
index bf9fa4e..f7ee6d2 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -136,11 +136,13 @@
async getStatus() {
const status = await this._getStatus();
- if (status.enabled && !this.isOnLatestPatchset(status.patchsetNumber)) {
- // status is outdated, abort and re-init
+ if (status.enabled && this._isOnOlderPatchset(status.patchsetNumber)) {
+ // status is returned for an older patchset. Abort, re-init and refetch
+ // new status - it is expected, that after several retry a status
+ // for the newest patchset is returned
this.reset();
this.prefetch();
- return await this.codeOwnersCacheApi.getStatus();
+ return await this.getStatus();
}
return status;
}
@@ -153,18 +155,21 @@
enabled: false,
codeOwnerStatusMap: new Map(),
rawStatuses: [],
+ newerPatchsetUploaded: false,
};
}
- const onwerStatus = await this.codeOwnersCacheApi.listOwnerStatus();
+ const ownerStatus = await this.codeOwnersCacheApi.listOwnerStatus();
return {
enabled: true,
- patchsetNumber: onwerStatus.patch_set_number,
+ patchsetNumber: ownerStatus.patch_set_number,
codeOwnerStatusMap: this._formatStatuses(
- onwerStatus.file_code_owner_statuses
+ ownerStatus.file_code_owner_statuses
),
- rawStatuses: onwerStatus.file_code_owner_statuses,
+ rawStatuses: ownerStatus.file_code_owner_statuses,
+ newerPatchsetUploaded:
+ this._isOnNewerPatchset(ownerStatus.patch_set_number),
};
}
@@ -274,9 +279,14 @@
});
}
- isOnLatestPatchset(patchsetId) {
+ _isOnNewerPatchset(patchsetId) {
const latestRevision = this.change.revisions[this.change.current_revision];
- return `${latestRevision._number}` === `${patchsetId}`;
+ return patchsetId > latestRevision._number;
+ }
+
+ _isOnOlderPatchset(patchsetId) {
+ const latestRevision = this.change.revisions[this.change.current_revision];
+ return patchsetId < latestRevision._number;
}
reset() {
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index 5fcbeec..6bc3465 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -66,17 +66,22 @@
<template is="dom-if" if="[[!_isLoading]]">
<template is="dom-if" if="[[!_pluginFailed(model.pluginStatus)]]">
<template is="dom-if" if="[[!model.branchConfig.no_code_owners_defined]]">
- <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
- <template is="dom-if" if="[[_overrideInfoUrl]]">
- <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]"
- target="_blank">
- <iron-icon icon="gr-icons:help-outline"
- title="Documentation for overriding code owners"></iron-icon>
- </a>
+ <template is="dom-if" if="[[!_newerPatchsetUploaded]]">
+ <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
+ <template is="dom-if" if="[[_overrideInfoUrl]]">
+ <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]"
+ target="_blank">
+ <iron-icon icon="gr-icons:help-outline"
+ title="Documentation for overriding code owners"></iron-icon>
+ </a>
+ </template>
+ <gr-button link on-click="_openReplyDialog">
+ [[_getSuggestOwnersText(_statusCount)]]
+ </gr-button>
</template>
- <gr-button link on-click="_openReplyDialog">
- [[_getSuggestOwnersText(_statusCount)]]
- </gr-button>
+ <template is="dom-if" if="[[_newerPatchsetUploaded]]">
+ <span>A newer patch set has been uploaded.</span>
+ </template>
</template>
<template is="dom-if" if="[[model.branchConfig.no_code_owners_defined]]">
<span>No code-owners file</span>
@@ -99,6 +104,7 @@
static get properties() {
return {
_statusCount: Object,
+ _newerPatchsetUploaded: Boolean,
_isLoading: {
type: Boolean,
computed: '_computeIsLoading(model.branchConfig, model.status, ' +
@@ -143,10 +149,12 @@
_onStatusChanged(status, userRole) {
if (!status || !userRole) {
this._statusCount = undefined;
+ this._newerPatchsetUploaded = undefined;
return;
}
const rawStatuses = status.rawStatuses;
this._statusCount = this._getStatusCount(rawStatuses);
+ this._newerPatchsetUploaded = status.newerPatchsetUploaded;
this.reporting.reportLifeCycle('owners-submit-requirement-summary-shown',
{...this._statusCount, user_role: userRole});
}
diff --git a/ui/owner-status-column.js b/ui/owner-status-column.js
index 8f57a2b..ae56d36 100644
--- a/ui/owner-status-column.js
+++ b/ui/owner-status-column.js
@@ -48,14 +48,18 @@
};
class BaseEl extends CodeOwnersModelMixin(Polymer.Element) {
- computeHidden(change, patchRange) {
- if ([change, patchRange].includes(undefined)) return true;
+ computeHidden(change, patchRange, newerPatchsetUploaded) {
+ if ([change, patchRange, newerPatchsetUploaded].includes(undefined)) {
+ return true;
+ }
// if code-owners is not a submit requirement, don't show status column
if (change.requirements &&
!change.requirements.find(r => r.type === 'code-owners')) {
return true;
}
+ if (newerPatchsetUploaded) return true;
+
const latestPatchset = change.revisions[change.current_revision];
// only show if its comparing against base
if (patchRange.basePatchNum !== 'PARENT') return true;
@@ -97,7 +101,8 @@
hidden: {
type: Boolean,
reflectToAttribute: true,
- computed: 'computeHidden(change, patchRange)',
+ computed: 'computeHidden(change, patchRange, ' +
+ 'model.status.newerPatchsetUploaded)',
},
};
}