Merge branch 'stable-3.10' into stable-3.11 * stable-3.10: API should take into account the configured approval label Fix GJF 1.7 formatting on java files Change-Id: Iaa0d99b8530e488f05f5973bbf001e390d889878
diff --git a/README.md b/README.md index 9c1decb..4ff322d 100644 --- a/README.md +++ b/README.md
@@ -1,43 +1,45 @@ # Gerrit OWNERS Plugin -This plugin provides some Prolog predicates that can be used to add customized -validation checks based on the approval of ‘path owners’ of a particular folder -in the project. +This repository comprises of effectively two separate plugins, `owners` and +`owners-autoassign`. -That allows creating a single big project including multiple components and -users have different roles depending on the particular path where changes are -being proposed. A user can be “owner” in a specific directory, and thus -influencing the approvals of changes there, but cannot do the same in others -paths, so assuring a kind of dynamic subproject access rights. +They share the ability to parse the same OWNERS file format, which facilitates +the maintenance of ACLs, as there is only one source of truth. -## How it works +For details on how to configure either plugin, please refer to the docs in the +specific plugin's folder. -There are currently two main prolog public verbs: +> **NOTE**: A comprehensive introduction to both `owners` and `owners-autoassign` has been +> given as part of the [GerritMeets series in Jan 2025](https://www.youtube.com/watch?v=NfIHnJF30Wo). -`add_owner_approval/3` (UserList, InList, OutList) -appends `label('Owner-Approval', need(_))` to InList building OutList if -UserList has no users contained in the defined owners of this path change. +Here's an introduction to both plugins: -In other words, the predicate just copies InList to OutList if at least one of -the elements in UserList is an owner. +## owners -`add_owner_approval/2` (InList, OutList) -appends `label('Owner-Approval', need(_))` to InList building OutList if -no owners has given a Code-Review +2 to this path change. +This plugin exposes the `has:approval_owners` predicate that can be used with +Gerrit's own +[submit-requirements](/Documentation/config-submit-requirements.html) to ensure +that a change has been approved by +the relevant users defined in the OWNERS file for the target branch of the +change. -This predicate is similar to the first one but generates a UserList with an -hardcoded policy. +This allows the creation of a single repository with multiple nested projects, each +potentially, used by different users/teams with different roles depending on the +particular path where changes are being proposed. A user can be “owner” in a +specific directory, thus influencing the approvals of changes there, but not +in others, enabling great flexibility when working on repositories shared by +multiple teams. -Since add_owner_approval/3 is not using hard coded policies, it can be suitable -for complex customizations. +## owners-autoassign -## Auto assigner +This plugin parses the same OWNERS file format as the owners plugin. It will +automatically assign all of the owners as reviewers to newly created or updated +changes. It also allows for completely custom management of the attention set, +i.e. allows, via custom integrations, to not add people on holiday to the +attention set, or that the same user is not added to too many changes at the +same time, etc... -There is a second plugin, gerrit-owners-autoassign which depends on -gerrit-owners. It will automatically assign all of the owners to review a -change when it's created or updated. - -## How to build +## Building the plugins This plugin is built with Bazel and two build modes are supported:
diff --git a/config.md b/config.md deleted file mode 100644 index 6cba6eb..0000000 --- a/config.md +++ /dev/null
@@ -1,283 +0,0 @@ -## Configuration - -Owner approval is determined based on OWNERS files located in the same -repository on the target branch of the changes uploaded for review. - -The `OWNERS` file has the following YAML structure: - -```yaml -inherited: true -owners: -- some.email@example.com -- User Name -- group/Group of Users -matchers: -- suffix: .java - owners: - [...] -- regex: .*/README.* - owners: - [...] -- partial_regex: example - owners: - [...] -- exact: path/to/file.txt - [...] -``` - -_NOTE: Be aware to double check that emails and full user names correspond to -valid registered Gerrit users. When owner user full name or e-mail cannot be -resolved, a corresponding WARN message is logged on Gerrit error_log and the -user entry dropped._ - -That translates to inheriting owner email address from any parent OWNER files -and to define 'some.email@example.com' or 'User Name' users as the mandatory -reviewers for all changes that include modification to those files. - -To specify a group of people instead of naming individual owners, prefix the -group name or UUID with 'group/'. - -Additional owners can be specified for files selected by other matching -conditions (matchers section). Matching can be done by file suffix, regex -(partial or full) and exact string comparison. For exact match, path is -relative to the root of the repo. - -The plugin analyzes the latest patch set by looking at each file directory and -building an OWNERS hierarchy. It stops once it finds an OWNERS file that has -“inherited” set to false (by default it’s true.) - -For example, imagine the following tree: - -``` -/OWNERS -/example/src/main/OWNERS -/example/src/main/java/com/example/foo/Foo.java -/example/src/main/resources/config.properties -/example/src/test/OWNERS -/example/src/test/java/com/example/foo/FooTest.java -``` - -If you submit a patch set that changes /example/src/main/java/com/example/foo/Foo.java -then the plugin will first open /example/src/main/OWNERS and if inherited is set -to true combine it with the owners listed in /OWNERS. - -If for each patch there is a reviewer who gave a Code-Review +2 then the plugin -will not add any labels, otherwise, it will add ```label('Code-Review from owners', need(_)).``` - -## Global project OWNERS - -Set a OWNERS file into the project refs/meta/config to define a global set of -rules applied to every change pushed, regardless of the folder or target branch. - -Example of assigning every configuration files to a specific owner group: - -```yaml -matchers: -- suffix: .config - owners: - - Configuration Managers -``` - -Global refs/meta/config OWNERS configuration is inherited only when the OWNERS file -contain the 'inherited: true' condition at the top of the file or if they are absent. - -That means that in the absence of any OWNERS file in the target branch, the refs/meta/config -OWNERS is used as global default. - - -## Example 1 - OWNERS file without matchers and default Gerrit submit rules - -Given an OWNERS configuration of: - -```yaml -inherited: true -owners: -- John Doe -- Doug Smith -``` - -And sample rules.pl that uses this predicate to enable the submit rule if -one of the owners has given a Code Review +2 - -```prolog -submit_rule(S) :- - gerrit:default_submit(D), - D =.. [submit | Ds], - findall(U, gerrit:commit_label(label('Code-Review', 2), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Then Gerrit would evaluate the Prolog rule as follows: - -It first gets the current default on rule which gives ok() if no Code-Review -2 -and at least a Code-Review +2 is being provided. - -Then it accumulates in Approvers the list of users who had given Code-Review +2 -and then checks if this list contains either 'John Doe' or 'Doug Smith'. - -If Approvers list does not include one of the owners, then Owner-Approval need() -is added thus making the change not submittable. - -## Example 2 - OWNERS file without matchers and no default Gerrit rules - -Given an OWNERS configuration of: - -```yaml -inherited: true -owners: -- John Doe -- Doug Smith -``` - -And a rule which makes submittable a change if at least one of the owners has -given a +1 without taking into consideration any other label: - -```prolog -submit_rule(S) :- - Ds = [ label(‘owners_plugin_default’,ok(user(100000))) ], - findall(U, gerrit:commit_label(label('Code-Review', 1), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Then Gerrit would make the change Submittable only if 'John Doe' or 'Doug Smith' -have provided at least a Code-Review +1. - -## Example 3 - OWNERS file without matchers and custom _Owner-Approves_ label - -Sometimes to differentiate the _owners approval_ on a change from the code -review on the entire project. The scenario could be for instance the sign-off of -the project's build dependencies based on the Company roles-and-responsibilities -matrix and governance process. - -In this case, we need to grant specific people with the _Owner-Approved_ label -without necessarily having to give Code-Review +2 rights on the entire project. - -Amend the project.config as shown in (1) and add a new label; then give -permissions to any registered user. Finally, define a small variant of the -Prolog rules as shown in (2). - -(1) Example fo the project config changes with the new label with values -(label name and values are arbitrary) - -``` -[label "Owner-Approved"] - function = NoOp - defaultValue = 0 - copyMinScore = true - copyAllScoresOnTrivialRebase = true - value = -1 I don't want this to be merged - value = 0 No score - value = +1 Approved -[access "refs/heads/*"] - label-Owner-Approved = -1..+1 group Registered Users -``` - -(2) Define the project's rules.pl with an amended version of Example 1: - -```prolog -submit_rule(S) :- - gerrit:default_submit(D), - D =.. [submit | Ds], - findall(U, gerrit:commit_label(label('Owner-Approved', 1), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Given now an OWNERS configuration of: - -```yaml -inherited: true -owners: -- John Doe -- Doug Smith -``` - -A change cannot be submitted until John Doe or Doug Smith add a label -"Owner-Approved", independently from being able to provide any Code-Review. - -## Example 4 - Owners based on matchers - -Often the ownership comes from the developer's skills and competencies and -cannot be defined solely by the project's directory structure. -For instance, all the files ending with .sql should be owned and signed-off by -the DBA while all the ones ending with .css by approved by the UX Team. - -Given an OWNERS configuration of: - -```yaml -inherited: true -matchers: -- suffix: .sql - owners: - - Mister Dba -- suffix: .css - owners: - - John Creative - - Matt Designer -``` - -And a rules.pl of: - -```prolog -submit_rule(S) :- - gerrit:default_submit(L), - L =.. [submit | Sr ], - gerrit_owners:add_match_owner_approval(Sr,A), - S =.. [submit | A ]. -``` - -Then for any change that contains files with .sql or .css extensions, besides -to the default Gerrit submit rules, the extra constraints on the additional -owners of the modified files will be added. The final submit is enabled if both -Gerrit default rules are satisfied and all the owners of the .sql files -(Mister Dba) and the .css files (either John Creative or Matt Designer) have -provided their Code-Review +2 feedback. - -The `add_match_owner_approval` predicate would also honour the OWNERS file -without matchers, giving, therefore, the possibility of having different ownership -criteria for different subdirectories. Example: /foo-dir/OWNERS can define a -directory-based ownership while /bar-dir/OWNERS can rely on matching rules. - -__PERFORMANCE NOTE: The predicate `add_match_owner_approval` looks, -at first sight, more powerful and versatile. However, it may generate a significant -number of reductions and therefore, impact the Gerrit server performance. -When used with changes with a high number of files involved, it may even crash -the Gerrit default `rules.reductionLimit`. -When not using any matcher in the OWNERS file, prefer the `add_owner_approval`, -which generates a minimal number of reductions.__ - -## Example 5 - Owners details on a per-file basis - -When using the owners with a series of matchers associated to different set of -owners, it may not be trivial to understand exactly *why* change is not approved -yet. - -We need to define one extra submit rule to scan the entire list of files in the -change and their associated owners and cross-check with the existing Code-Review -feedback received. - -Given the same OWNERS and rules.pl configuration of Example 4 with the following -extra rule: - -```prolog -submit_rule(submit(W)) :- - gerrit_owners:findall_match_file_user(W). -``` - -For every change that would include any .sql or .css file (e.g. my-update.sql -and styles.css) Gerrit will display as additional description on the "need" code -review labels section of the change screen: - -``` -Code-Review from owners -Mister Dba owns my-update.sql -John Creative, Matt Designer own styles.css -``` - -As soon as the owners reviews are provided, the corresponding entry will be -removed from the "need" section of the change. - -In this way, it is always clear which owner needs to provide their feedback on -which file of the change.
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/SyncReviewerManager.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/SyncReviewerManager.java index bdfa7f6..e9acc43 100644 --- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/SyncReviewerManager.java +++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/SyncReviewerManager.java
@@ -140,13 +140,15 @@ } in.ignoreAutomaticAttentionSetRules = true; - in.addToAttentionSet = - ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewersAccounts).stream() - .map( - (reviewer) -> - new AttentionSetInput( - reviewer.toString(), "Selected as member of the OWNERS file")) - .collect(Collectors.toList()); + if (ownersForAttentionSet != null) { + in.addToAttentionSet = + ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewersAccounts).stream() + .map( + (reviewer) -> + new AttentionSetInput( + reviewer.toString(), "Selected as member of the OWNERS file")) + .collect(Collectors.toList()); + } gApi.changes().id(changeInfo.id).current().review(in); }
diff --git a/owners-autoassign/src/main/resources/Documentation/attention-set.md b/owners-autoassign/src/main/resources/Documentation/attention-set.md index 0ae7fa5..ca4295f 100644 --- a/owners-autoassign/src/main/resources/Documentation/attention-set.md +++ b/owners-autoassign/src/main/resources/Documentation/attention-set.md
@@ -7,20 +7,6 @@ quite long, due to the hierarchy of the OWNERS files in the parent directories. -The `owners-api.jar` libModule included in the owners' plugin project contains -a generic interface that can be used to customize Gerrit's default -attention-set behaviour. - -## owners-api setup - -Copy the `owners-api.jar` libModule into the $GERRIT_SITE/lib directory -and add the following entry to `gerrit.config`: - -``` -[gerrit] - installModule = com.googlesource.gerrit.owners.api.OwnersApiModule -``` - ## Customization of the attention-set selection The OwnersAttentionSet API, contained in the owners-api.jar libModule,
diff --git a/owners-autoassign/src/main/resources/Documentation/config.md b/owners-autoassign/src/main/resources/Documentation/config.md index 1caf92f..0353861 100644 --- a/owners-autoassign/src/main/resources/Documentation/config.md +++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -1,13 +1,3 @@ -## Setup - -The owners-autoassign plugin depends on the shared library `owners-api.jar` -which needs to be installed into the `$GERRIT_SITE/lib` and requires a -restart of the Gerrit service. - -Once the `owners-api.jar` is loaded at Gerrit startup, the `owners-autoassign.jar` -file can be installed like a regular Gerrit plugin, by being dropped to the -`GRRIT_SITE/plugins` directory or installed through the plugin manager. - ## Global configuration The global plugin configuration is read from the `$GERRIT_SITE/etc/owners-autoassign.config`
diff --git a/owners-autoassign/src/main/resources/Documentation/setup.md b/owners-autoassign/src/main/resources/Documentation/setup.md new file mode 100644 index 0000000..b17f851 --- /dev/null +++ b/owners-autoassign/src/main/resources/Documentation/setup.md
@@ -0,0 +1,16 @@ +## Setup + +The owners-autoassign plugin depends on the shared library `owners-api.jar` +which needs to be installed into the `$GERRIT_SITE/lib`. You will then need to +add the following entry to `gerrit.config`: + +``` +[gerrit] + installModule = com.googlesource.gerrit.owners.api.OwnersApiModule +``` + +A restart of the Gerrit service will be required for the lib to be loaded. + +Once the `owners-api.jar` is loaded at Gerrit startup, the `owners-autoassign.jar` +file can be installed like a regular Gerrit plugin, by simply placing the file +in the `GERRIT_SITE/plugins` directory or installed through the plugin manager. \ No newline at end of file
diff --git a/owners/src/main/resources/Documentation/config.md b/owners/src/main/resources/Documentation/config.md index 4baa2c5..4b94ac3 100644 --- a/owners/src/main/resources/Documentation/config.md +++ b/owners/src/main/resources/Documentation/config.md
@@ -44,9 +44,10 @@ <a name="owners.enableSubmitRequirement">owners.enableSubmitRequirement</a> : If set to `true` the approvals are evaluated through the owners plugin - provided submit rule without a need of prolog predicate being added to a - project or submit requirement configured in the `project.config` as it is - automatically applied to all projects. Defaults to `false`. + default submit requirement, named "Code-Review-from-Owners", without a need of + prolog predicate being added to a project or submit requirement configured + in the `project.config` as it is automatically applied to all projects. + Defaults to `false`. Example: @@ -71,7 +72,7 @@ > Please also note, that project's `rules.pl` should be removed in this > case so that it doesn't interfere with a change evaluation. > - > The minial configuration looks like below (see + > The minimal configuration looks like below (see > [submit requirements documentation](/Documentation/config-submit-requirements.html) for more details): > ``` > [submit-requirement "Owner-Approval"] @@ -162,7 +163,7 @@ > configuration is used) so that owners don't have to be granted with the > maximum label's score. Note that only single digit (0..9) is allowed. -For example, imagine the following tree: +For example, imagine the following tree with a default Gerrit project labels configuration: ``` /OWNERS @@ -203,7 +204,7 @@ If the global project OWNERS has the 'inherited: true', it will check for a global project OWNERS in all parent projects up to All-Projects. -## Example 1 - OWNERS file without matchers and default Gerrit submit rules +## Example 1 - OWNERS file without matchers Given an OWNERS configuration of: @@ -214,128 +215,34 @@ - Doug Smith ``` -### When `owners.enableSubmitRequirement = true` +In this case the owners plugin will assume the default label configuration,`Code-Review ++2`, as indication of approval. -Then Gerrit would: - -Evaluate default submit requirement which gives `OK` if no `Code-Review -2` is -given and at least one `Code-Review +2` is being provided. - -Evaluate owners submit requirement to check if `Code-Review +2` is given by -either 'John Doe' or 'Doug Smith'. If none of them has approved then -`Code-Review from owners` requirement is added making the change not -submittable. +To enforce submittability only when the specified owners have approved the +change, you can then either enable `owners.enableSubmitRequirement = true` in +your `gerrit.config` or define a submit requirement in your `project.config` that +uses the `has:approval_owners` in the `submittableIf` section, like so: +``` +[submit-requirement "Owner-Approval"] + description = Files needs to be approved by owners + submittableIf = has:approval_owners +``` > See [notes](#owners.enableSubmitRequirement) for an example on how to enable > submit requirements for a specific project only. -### When `owners.enableSubmitRequirement = false` (default) +## Example 2 - OWNERS file with custom approval label -And sample rules.pl that uses this predicate to enable the submit rule if -one of the owners has given a Code Review +2 +Sometimes, teams might wish to use a different label to signal approval from an +owner, rather than the default `Code-Review +2`. For example, you might wish to +have a governance team explicitly approving changes but do not wish to necessarily grant +them code review permissions. -```prolog -submit_rule(S) :- - gerrit:default_submit(D), - D =.. [submit | Ds], - findall(U, gerrit:commit_label(label('Code-Review', 2), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Then Gerrit would evaluate the Prolog rule as follows: - -It first gets the current default on rule which gives ok() if no Code-Review -2 -and at least a Code-Review +2 is being provided. - -Then it accumulates in Approvers the list of users who had given Code-Review +2 -and then checks if this list contains either 'John Doe' or 'Doug Smith'. - -If Approvers list does not include one of the owners, then Owner-Approval need() -is added thus making the change not submittable. - -## Example 2 - OWNERS file without matchers and no default Gerrit rules - -Given an OWNERS configuration of: - -```yaml -inherited: true -owners: -- John Doe -- Doug Smith -``` - -### When `owners.enableSubmitRequirement = true` - -This case is supported with the `Code-Review` label and `OWNERS` file -modifications. - -The `OWNERS` file requires the label configuration to be added (here is the -updated version): - -```yaml -inherited: true -label: Code-Review, 1 -owners: -- John Doe -- Doug Smith -``` - -But additionally one needs to modify the label on the particular project level -to the following version: - -``` -[label "Code-Review"] - function = NoOp - defaultValue = 0 - copyMinScore = true - copyAllScoresOnTrivialRebase = true - value = -2 This shall not be merged - value = -1 I would prefer this is not merged as is - value = 0 No score - value = +1 Looks good to me, but someone else must approve - value = +2 Looks good to me, approved -``` - -Note that `function` is set to `NoOp`. - -As a result Gerrit would make the change Submittable only if 'John Doe' or -'Doug Smith' have provided at least a `Code-Review +1`. - -> See [notes](#owners.enableSubmitRequirement) for an example on how to enable -> submit requirements for a specific project only. - -### When `owners.enableSubmitRequirement = false` (default) - -And a rule which makes submittable a change if at least one of the owners has -given a +1 without taking into consideration any other label: - -```prolog -submit_rule(S) :- - Ds = [ label(‘owners_plugin_default’,ok(user(100000))) ], - findall(U, gerrit:commit_label(label('Code-Review', 1), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Then Gerrit would make the change Submittable only if 'John Doe' or 'Doug Smith' -have provided at least a Code-Review +1. - -## Example 3 - OWNERS file without matchers and custom _Owner-Approves_ label - -Sometimes to differentiate the _owners approval_ on a change from the code -review on the entire project. The scenario could be for instance the sign-off of -the project's build dependencies based on the Company roles-and-responsibilities -matrix and governance process. - -In this case, we need to grant specific people with the _Owner-Approved_ label -without necessarily having to give Code-Review +2 rights on the entire project. +In this case, we need to grant specific groups with a custom label without +necessarily giving them Code-Review +2 rights on the entire project. Amend the project.config as shown in below and add a new label; then give -permissions to any registered user. - -Example fo the project config changes with the new label with values -(label name and values are arbitrary) +permissions to any registered user: ``` [label "Owner-Approved"] @@ -350,82 +257,45 @@ label-Owner-Approved = -1..+1 group Registered Users ``` -### When `owners.enableSubmitRequirement = true` - Given now an OWNERS configuration of: ```yaml inherited: true -label: Owner-Approved +label: Owner-Approved, 1 owners: - John Doe - Doug Smith ``` -A change cannot be submitted until 'John Doe' or 'Doug Smith' add a label -`Owner-Approved`, independently from being able to provide any Code-Review. +This will mean that, a change cannot be submitted until 'John Doe' or 'Doug +Smith' vote +1 on the `Owner-Approved` label, independently from whomever else +has provided a `Code-Review +2`. + +> **NOTE:** You can also use `label: Code-Review, 1` as your definition of submittability + +Finally to enable this, you'll need to define a custom submit requirement in +your `project.config`(as above) or enable `owners.enableSubmitRequirement = true` in your +`owners.config` + +If you no longer wish to require a `Code-Review +2` and would rather only use +the custom submit requirement, you have two options: +- change the definition of the `Code-Review` label in `All-Projects`'s + `project.config` so that `function = NoOp`. +- set an `overrideIf` clause in your custom submit requirement definition > See [notes](#owners.enableSubmitRequirement) for an example on how to enable > submit requirements for a specific project only. -### When `owners.enableSubmitRequirement = false` (default) +> See [notes](#label.Label-Name.function) for an example on how to modify +> label functions, as by default `Code-Review` requires at least one max vote +> from any user. -Finally, define prolog rules as shown in below (an amended version of -Example 1): - -```prolog -submit_rule(S) :- - gerrit:default_submit(D), - D =.. [submit | Ds], - findall(U, gerrit:commit_label(label('Owner-Approved', 1), U), Approvers), - gerrit_owners:add_owner_approval(Approvers, Ds, A), - S =.. [submit | A]. -``` - -Given now an OWNERS configuration of: - -```yaml -inherited: true -owners: -- John Doe -- Doug Smith -``` - -A change cannot be submitted until John Doe or Doug Smith add a label -"Owner-Approved", independently from being able to provide any Code-Review. - -## Example 4 - OWNERS file without matchers, default Gerrit submit rules and owners Code-Review +1 required - -This is a variant of `example 3` when no additional label is created but owners -shouldn't be granted with `Code-Review +2` for all project files as it might be -outside of their comptenence/comfort zone. - -### When `owners.enableSubmitRequirement = true` - -Given an OWNERS configuration of: - -```yaml -inherited: true -label: Code-Review, 1 -owners: -- John Doe -- Doug Smith -``` - -A change cannot be submitted until 'John Doe' or 'Doug Smith' add -`Code-Review+1` score. Note that regular developers still need to cast the -`Code-Review+2` for a change to be submittable as default submit rule is -still evaluated. - -> See [notes](#owners.enableSubmitRequirement) for an example on how to enable -> submit requirements for a specific project only. - -## Example 5 - Owners based on matchers +## Example 3 - Owners based on matchers Often the ownership comes from the developer's skills and competencies and cannot be purely defined by the project's directory structure. For instance, all the files ending with .sql should be owned and signed-off by -the DBA while all the ones ending with .css by approved by the UX Team. +the DBA while all the ones that contain 'Test' should be reviewed by the QA team. Given an OWNERS configuration of: @@ -435,81 +305,15 @@ - suffix: .sql owners: - Mister Dba -- suffix: .css +- regex: .*Test.* owners: - - John Creative - - Matt Designer + - John Bug + - Matt Free ``` -### When `owners.enableSubmitRequirement = true` - -Then for any change that contains files with .sql or .css extensions, besides -to the default Gerrit submit rules, the extra constraints on the additional -owners of the modified files will be added. The final submit is enabled if both -Gerrit default rules are satisfied and all the owners of the .sql files -('Mister Dba') and the .css files (either 'John Creative' or 'Matt Designer') -have provided their `Code-Review +2` feedback (as `Code-Review` is default -label for owners submit requirement). +You can then either enable `owners.enableSubmitRequirement = true` in your +`gerrit.config` or define a submit requirement in your `project.config` that +uses the `has:approval_owners` in the `applicableIf` section. > See [notes](#owners.enableSubmitRequirement) for an example on how to enable > submit requirements for a specific project only. - -### When `owners.enableSubmitRequirement = false` (default) - -And a rules.pl of: - -```prolog -submit_rule(S) :- - gerrit:default_submit(L), - L =.. [submit | Sr ], - gerrit_owners:add_match_owner_approval(Sr,A), - S =.. [submit | A ]. -``` - -Then for any change that contains files with .sql or .css extensions, besides -to the default Gerrit submit rules, the extra constraints on the additional -owners of the modified files will be added. The final submit is enabled if both -Gerrit default rules are satisfied and all the owners of the .sql files -(Mister Dba) and the .css files (either John Creative or Matt Designer) have -provided their Code-Review +2 feedback. - -## Example 6 - Owners details on a per-file basis - -### When `owners.enableSubmitRequirement = true` - -This case is obsolete and _only_ prolog specific. The list of which file is -owned by whom is available through the [REST API](rest-api.md). - -### When `owners.enableSubmitRequirement = false` (default) - -When using the owners with a series of matchers associated to different set of -owners, it may not be trivial to understand exactly *why* change is not approved -yet. - -We need to define one extra submit rule to scan the entire list of files in the -change and their associated owners and cross-check with the existing Code-Review -feedback received. - -Given the same OWNERS and rules.pl configuration of Example 4 with the following -extra rule: - -```prolog -submit_rule(submit(W)) :- - gerrit_owners:findall_match_file_user(W). -``` - -For every change that would include any .sql or .css file (e.g. my-update.sql -and styles.css) Gerrit will display as additional description on the "need" code -review labels section of the change screen: - -``` -Code-Review from owners -Mister Dba owns my-update.sql -John Creative, Matt Designer own styles.css -``` - -As soon as the owners reviews are provided, the corresponding entry will be -removed from the "need" section of the change. - -In this way, it is always clear which owner needs to provide their feedback on -which file of the change.
diff --git a/owners/src/main/resources/Documentation/how-to-use.md b/owners/src/main/resources/Documentation/how-to-use.md index e539d65..b7f0292 100644 --- a/owners/src/main/resources/Documentation/how-to-use.md +++ b/owners/src/main/resources/Documentation/how-to-use.md
@@ -12,19 +12,22 @@ ## Context -The owners plugin can be used in three different modes: -1. Prolog rules only(deprecated since Gerrit 3.6). -2. With plugin-provided submit requirements. -3. With user defined custom submit requirements. +The owners plugin can be used in two different modes: +1. With plugin-provided submit requirements. +2. With user defined custom submit requirements. -When using the plugin in mode 1. the functionality is limited to the generation -of a Prolog-based submit rule with no extra UI features. +> **NOTE**: Previous to 3.6 it was also possible to use the plugin with Prolog rules, +> while this has not yet completely stopped working, the functionalities of Prolog +> rules are now strongly limited and only applied on a best effort basis. It's +> highly advised to completely remove them from your system as a matter of +> priority. +> To add, when using Prolog rules there are no UI features. -On top of providing significantly better and more predictable performances, -using the plugin in either mode 2. or 3. provides extra capabilities like: -- A REST-api that exposes the owners approval status with single file granularity. -- Enhanced UI experience with much clearer explanation of who owns what files, - as explained below. +> On top of providing significantly better and more predictable performances, +> using the plugin in either mode 1. or 2. provides extra capabilities like: +> - A REST-api that exposes the owners approval status with single file granularity. +> - Enhanced UI experience with much clearer explanation of who owns what files, +> as explained below. > **NOTE**: The owners plugin uses multiple subsystems of Gerrit for determining > the status of the submit requirement predicates or Prolog functions. @@ -94,7 +97,7 @@ ### <a id="ownerStatus.submitRule">Owners status for submit rule - `OWNERS` file can still be evaluated (without a need for the prolog +`OWNERS` file can still be evaluated (without a need for the Prolog predicate being added to the project) by enabling the [default submit requirements](config.html#owners.enableSubmitRequirement). In this way, results of the `OWNERS` file evaluation are provided to @@ -107,9 +110,9 @@  - > Note that in replacement mode, owner's votes are displayed next to the - > `Code Review from owners` submit requirement instead of its status. - > It is a Gerrit's core representation of submit rule. + > Note that when default submit requirements are enabled, owner's votes are + > displayed next to the `Code Review from owners` submit requirement instead + > of its status. It is a Gerrit's core representation of submit rule. * Next to all owned files (when the newest patchset is selected) \ @@ -120,8 +123,8 @@ #### <a id="ownersStatus.submitRule.owners">`Code Review from owners` submit requirement -The `Code Review from owners` (that is available only in -[replacement mode](config.html#owners.enableSubmitRequirement)) submit +The `Code Review from owners` (i.e. the +[default submit requirement](config.html#owners.enableSubmitRequirement)) submit requirement provides general information if all owned files were approved (requirement is satisfied). When hovered, a detailed description is shown @@ -129,8 +132,9 @@ #### <a id="ownersStatus.submitRule.files">Per file owners statuses -The owners status per file in replacement mode doesn't differ from -[submit requirements mode](#ownersStatus.submitRequirement.files). +The owners status per file when [default submit +requirements](config.html#owners.enableSubmitRequirement) are enabled doesn't +differ from [submit requirements mode](#ownersStatus.submitRequirement.files). ### Files that are assigned to the current user based on the OWNERS
diff --git a/owners/src/main/resources/Documentation/metrcis.md b/owners/src/main/resources/Documentation/metrics.md similarity index 100% rename from owners/src/main/resources/Documentation/metrcis.md rename to owners/src/main/resources/Documentation/metrics.md
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersRegressionIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersRegressionIT.java index 2fc37ce..1f23510 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersRegressionIT.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersRegressionIT.java
@@ -72,19 +72,24 @@ private void verifySubmitRequirement( Collection<SubmitRequirementResultInfo> requirements, String name, Status status) { - assertThat(requirements).hasSize(1); + // in post stable-3.10 Gerrit has default submit requirement no-unresolved-comments turned on by + // default therefore 2 requirements will be always present + assertThat(requirements).hasSize(2); - SubmitRequirementResultInfo requirement = requirements.iterator().next(); - if (!requirement.name.equals(name) || !(requirement.status == status)) { - throw new AssertionError( - String.format( - "No submit requirement %s with status %s (existing = %s)", - name, - status, - requirements.stream() - .map(r -> String.format("%s=%s", r.name, r.status)) - .collect(toImmutableList()))); + for (SubmitRequirementResultInfo requirement : requirements) { + if (requirement.name.equals(name) && requirement.status == status) { + return; + } } + + throw new AssertionError( + String.format( + "Could not find submit requirement %s with status %s (results = %s)", + name, + status, + requirements.stream() + .map(r -> String.format("%s=%s", r.name, r.status)) + .collect(toImmutableList()))); } private ChangeApi forChange(PushOneCommit.Result r) throws RestApiException {
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java index 3db9dcb..ba35cdb 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/OwnersSubmitRequirementITAbstract.java
@@ -88,7 +88,7 @@ assertThat(changeNotReady.requirements).isEmpty(); changeApi.current().review(ReviewInput.approve()); - ChangeInfo changeReady = changeApi.get(); + ChangeInfo changeReady = forChange(r).get(); assertThat(changeReady.submittable).isTrue(); assertThat(changeReady.requirements).isEmpty(); } @@ -157,9 +157,10 @@ forChange(r).get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.NEED); changeApi.current().review(new ReviewInput().label(LabelId.VERIFIED, 1)); - assertThat(changeApi.get().submittable).isTrue(); + ChangeInfo changeReady = forChange(r).get(); + assertThat(changeReady.submittable).isTrue(); verifyHasSubmitRecord( - changeApi.get().submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK); + changeReady.submitRecords, LabelId.VERIFIED, SubmitRecordInfo.Label.Status.OK); } @Test @@ -269,7 +270,7 @@ requestScopeOperations.setApiUser(admin2.id()); forChange(r).current().review(ReviewInput.recommend()); - ChangeInfo ownersVoteNotSufficient = changeApi.get(); + ChangeInfo ownersVoteNotSufficient = forChange(r).get(); assertThat(ownersVoteNotSufficient.submittable).isFalse(); verifyChangeReady(ownersVoteNotSufficient); verifyHasSubmitRecord(