Obsolete
Status Update
Comments
al...@google.com <al...@google.com> #2
[Empty comment from Monorail migration]
al...@google.com <al...@google.com> #3
[Empty comment from Monorail migration]
da...@gmail.com <da...@gmail.com> #4
[Empty comment from Monorail migration]
da...@gmail.com <da...@gmail.com> #5
As far as I remember, one of the reasons we documented ES as experimental was because none of us maintainers were/are using it in production.
I've heard that there are some users running ES in production now though. Would it be good to do a survey of the community to find out which versions (both of ES and of Gerrit) are actually being used in the wild? Is this something that the community managers can drive?
[Monorail components: Community]
I've heard that there are some users running ES in production now though. Would it be good to do a survey of the community to find out which versions (both of ES and of Gerrit) are actually being used in the wild? Is this something that the community managers can drive?
[Monorail components: Community]
th...@gmail.com <th...@gmail.com> #6
I think so yes; thanks. Now part of our near backlog.
There could be other Gerrit components to poll about; cf.https://crbug.com/gerrit/12211 .
There could be other Gerrit components to poll about; cf.
al...@google.com <al...@google.com> #7
Make a dedicated short survey for this to get this resolved quickly.
da...@gmail.com <da...@gmail.com> #8
[Empty comment from Monorail migration]
sv...@axis.com <sv...@axis.com> #9
Form proposal sent for review to some of us here.
Let me know if you cannot see it but would like to.
Once done reviewing, I can send the form/survey to the community mailing list.
-How much time to allow for collecting responses?
I'd propose by end of May, to give organizations a chance at figuring that out for themselves.
Let me know if you cannot see it but would like to.
Once done reviewing, I can send the form/survey to the community mailing list.
-How much time to allow for collecting responses?
I'd propose by end of May, to give organizations a chance at figuring that out for themselves.
sv...@axis.com <sv...@axis.com> #10
Looks good. I have just a couple of suggestions:
- Expand on the "both discontinued" statement to make it clearer what is meant. This is referring to the fact that those ES versions have reached EOL upstream, and support for them has been discontinued in Gerrit.
- I think it would be useful to also ask which version of Gerrit is being used. Then we can make a better judgement about whether it's safe to discontinue ES support in Gerrit's stable releases. Additionally it would help to decide in which stable branch we're going to remove the "experimental" caveat from the documentation.
- Expand on the "both discontinued" statement to make it clearer what is meant. This is referring to the fact that those ES versions have reached EOL upstream, and support for them has been discontinued in Gerrit.
- I think it would be useful to also ask which version of Gerrit is being used. Then we can make a better judgement about whether it's safe to discontinue ES support in Gerrit's stable releases. Additionally it would help to decide in which stable branch we're going to remove the "experimental" caveat from the documentation.
da...@gmail.com <da...@gmail.com> #11
Thanks David.
- I added a "Discontinued Elasticsearch versions" section from your first suggestion above, with a few more words and upstream reference.
- I added 2 questions based on your second suggestion; good point as well.
I also added a trailing note about the survey end date (May end).
And I added Edwin's email suggestions to the survey intro.
I altered some questions to fit the "no Elasticsearch use" case in; thanks Edwin.
Can one of the form reviewer(s) test it, or I can send it as is; let me know.
- I added a "Discontinued Elasticsearch versions" section from your first suggestion above, with a few more words and upstream reference.
- I added 2 questions based on your second suggestion; good point as well.
I also added a trailing note about the survey end date (May end).
And I added Edwin's email suggestions to the survey intro.
I altered some questions to fit the "no Elasticsearch use" case in; thanks Edwin.
Can one of the form reviewer(s) test it, or I can send it as is; let me know.
sv...@axis.com <sv...@axis.com> #12
The form looks good now. I think it's OK to send it, but let's wait for comments from anyone else.
sv...@axis.com <sv...@axis.com> #13
I just realized that I forgot David (O.), so I just invited him to the form.
I'm done receiving go-aheads from everyone else by now.
I'm then planning to post the survey on Monday morning my time.
-This is so that I
1. can check David-O's comments if any, and
2. skip the lower email attendance /week-end.
I'm done receiving go-aheads from everyone else by now.
I'm then planning to post the survey on Monday morning my time.
-This is so that I
1. can check David-O's comments if any, and
2. skip the lower email attendance /week-end.
sv...@axis.com <sv...@axis.com> #14
> I just realized that I forgot David (O.), so I just invited him to the form.
Thanks, looks good. I have not found a way how to comment on the
Form document, answering here:
Can we add additional question if ES not used currently:
Are your organization considering/planning to switch to using Elasticsearch in near future?
Possible further question if asnwer to the above question is Yes/No:
Yes:
What specific version?
No:
Why not?
Thanks, looks good. I have not found a way how to comment on the
Form document, answering here:
Can we add additional question if ES not used currently:
Are your organization considering/planning to switch to using Elasticsearch in near future?
Possible further question if asnwer to the above question is Yes/No:
Yes:
What specific version?
No:
Why not?
is...@google.com <is...@google.com> #15
Thanks David. -Done.
Survey form is posted to the list now; awaiting responses...
We will monitor upcoming responses until May end.
I'm setting this Issue to Started under my name until then.
Responses should teach us how to tackle versions and beta-ness of ES in Gerrit.
Survey form is posted to the list now; awaiting responses...
We will monitor upcoming responses until May end.
I'm setting this Issue to Started under my name until then.
Responses should teach us how to tackle versions and beta-ness of ES in Gerrit.
ae...@gmail.com <ae...@gmail.com> #16
Comment has been deleted.
Description
*General feature suitability*
In general, automatic reviewer addition would be a desired feature for Gerrit and nicely extend Gerrit's existing feature set. It's imaginable that many users would benefit from it. Hence, having such a functionality as core plugin or even directly in core would make sense.
This doesn't cover additional functionality related to reviewers, though.
*Comparison with other plugins*
The original request named 'reviewers-by-blame', 'reviewerassistant', 'find-owners', and 'owners' as similar plugins.
* 'find-owners' and 'owners' have a different focus as they enforce specific approvals. Any automatic reviewer addition by them would be just a nice-to-have.
* 'reviewers-by-blame' seems to be very specific. As the request pointed out, the 'reviewers' plugin could be extended in the future to also support other ways to identify suitable reviewers for automatic addition.
* 'reviewassistant' would have many overlapping features with 'reviewers' as it also has a strong part about automatically adding reviewers. It even seems to offer different modes and considerations for it, which seems nice. On the other hand, that plugin is developed against Gerrit v2.10-rc0 according to its documentation and doesn't support other versions. Furthermore, 'reviewassistant' contains functionality unrelated to reviewer addition. That functionality part might also not be desired by all users.
*Feature and REST API improvement areas for the 'reviewers' plugin*
There are some aspects which would need to be clarified and fixed/adjusted before we can make 'reviewers' a core plugin.
1) The 'reviewers' plugin allows to switch to a reviewer-suggestion mode instead. This kind of functionality brings in a totally different aspect to the plugin compared to the otherwise reviewer-addition focus. We don't want to mix those two aspects at the moment. The 'reviewers' plugin should only focus on reviewer addition, which is a big enough area in its own. Any reviewer suggestions should be left out.
2) The 'reviewers' plugin should work nicely together with existing core Gerrit functionality. This is not always given.
2a) WIP changes shouldn't get reviewers added on upload. Only when a change switches out of the WIP state, reviewers should be automatically added and notified. For that case, it might also be desirable to show to users an indication that reviewers will be automatically added when they actively leave the WIP state on the UI. This is the default behavior the plugin should offer. If there are known use cases for a different desired behavior for WIP states, it would be good to document them so that we can consider whether offering a configuration option for another behavior would make sense.
2b) Private changes shouldn't get reviewers added automatically, not even via a config option. As private changes deliberately have a limited visibility, automatic reviewer addition could impose a risk, especially as the added reviewers are not transparent for a person uploading a change. To ensure that appropriate approvals/reviews are given for private changes, another approach (e.g. labels) has to be chosen. Ensuring appropriate approvals/reviews is not considered a responsibility of the 'reviewers' plugin. When a private change is made public, the automatic reviewer addition should kick in similarly as with switching out of the WIP state (-> ideally also with a UI indication).
3) The group support and behavior is not clear. Are only Gerrit internal groups supported? Is the name in the configuration as mentioned in the documentation the one from the groups file or the actual name of the group? If the latter, is renaming of groups considered? Is specification of other group identifiers (e.g. UUIDs) blocked? How can groups be distinguished from accounts? Will groups fully expand to their members? Will the expansion also happen transitively? What happens in case of very large groups? If a large number of reviewers is added due to such groups, it wouldn't be acceptable behavior, especially if such additions aren't and can't be limited. Overall, support for groups would either need to be polished or removed.
4) The documentation claims that email addresses or usernames can be used for specifying accounts. Are the other options (e.g. full name) blocked? If a user changes their username or email address, how will/can all occurrences in the 'reviewers' configuration be updated? Would this be manual or automatic? In the light of this, it might also make sense to switch to a group-only configuration as Gerrit deliberately only works with groups in other parts (e.g. permissions) to avoid issues related to persons changing names/identities.
5) The REST API of the plugin seems to be for configuration purposes. It's not clear why it's possible to deactivate the REST API of the plugin, especially as it would be necessary to support the UI part of the plugin.
6) Modification of reviewer configurations seems to be possible for project owners and people having a dedicated, global permission. The use cases for the global permission are not clear.
7) According to the documentation, only the PUT REST endpoint seems to be guarded by permissions. Having the GET REST endpoint completely open without any permissions could impose a risk, especially as groups and accounts could be mentioned which are not visible to all users.
8) Using just the name of the plugin as name for the REST API endpoints doesn't seem ideal. By having the endpoints "GET/PUT /projects/<project-name>/reviewers", the endpoints are claiming to be generally about reviewers, which they currently aren't. The focus of the 'reviewers' plugin is assumed to be on automatic reviewer addition. If so, the endpoints should reflect that by getting a different, targeted name. If the plugin and those endpoints are instead designed to cover various reviewer-related aspects (e.g. also suggestions), we'd need to evaluate the core plugin request in a different light.
9) The current REST API endpoints don't follow Gerrit's model for REST resources. For instance, a PUT would mean a complete update of a REST resource. The current PUT endpoint should be a POST endpoint instead. Furthermore, it's strange to use the same REST endpoint name for PUT/POST and GET without mirroring input and output. If those endpoints are not a simple write and read of each other, they should have different names. In addition, they should be updated to follow the REST-resource-model-style. Furthermore, the usage of the 'action' type fields seems to be not custom for Gerrit. If there is a core functionality which was used as inspiration, it would be good to know. If this is a new way, we should discuss the approach.
10) Inheritance of the configured default reviewers and interaction with the offered REST endpoints is not clear. Apparently, the configuration can be inherited from parent projects. Would the GET REST endpoint also mention the inherited default reviewers? Would the PUT/POST endpoint also allow to modify those? If no for both of them (which should be documented), does the UI at least point to the parent project configuration via a link as Gerrit currently does it for ACL configuration?
11) Is there a way to validate the configured 'filters' for valid queries? Is this done automatically on update? What are the effects of invalid configurations?