Re: Who merges Merge Requests, and when?
On 10/06/2020 18:19, Glen Ditchfield wrote: In the Phabricator work flow, I would submit a patch, someone knowledgeable would accept it, and I would `arc land` it. Clear and simple. GitLab doesn't have that "accept" step AFAIK. The KDE Wiki's Infrastructure/ GitLab page says "Once the Merge Request is accepted, KDE Developers will merge it for you!" That is imprecise; I'm in the Developer group, but I'm just a casual. AFAIU, with GitLab "accept" could to be, "+1", "ship it", "merge it", I've seen all three used, I've also seen a reviewer just committing/merging the change, that counts as accept too :) Also note that if there are any "unresolved" threads in the MR, the merge button will be disabled, so this sort of acts like Phabricator "this review requires changes to proceed". So, I have a couple of MRs in flight: https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/1 hasn't attracted any review comments. I don't think I _should_ merge it, Exactly. but I think I _can_, which makes me uneasy. As has been said, anyone with a developer account could/can push to pretty much most repos in KDE. Having one's code reviewed before committing is always good because it's possible one might have missed something. You can use @user in a comment so that user/dev gets a notification/email to get their attention, then "ping" (maybe 1-2 times), then maybe post to this ML, or ask on IRC. I know it's more work, but apparently there are always more review requests than there are devs to review them :D https://invent.kde.org/pim/kcalutils/-/merge_requests/5 had comments, which I resolved. So, do I merge it? Do I wait for both reviewers to upvote it? Does a reviewer merge it? Personally, I'd wait for one of the reviewers to merge it; but it happens that sometimes a reviewer has already said "change this and that then merge it" in which case, you can go ahead and merge it. I hope that helps (and that my understanding of the issue isn't wrong :)). -- Ahmad Samir
Re: Who merges Merge Requests, and when?
El dimecres, 10 de juny de 2020, a les 18:19:36 CEST, Glen Ditchfield va escriure: > In the Phabricator work flow, I would submit a patch, someone knowledgeable > would accept it, and I would `arc land` it. Clear and simple. > > GitLab doesn't have that "accept" step AFAIK. The KDE Wiki's Infrastructure/ > GitLab page says "Once the Merge Request is accepted, KDE Developers will > merge it for you!" That is imprecise; I'm in the Developer group, but I'm > just a casual. > > So, I have a couple of MRs in flight: > > https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/1 hasn't > attracted any review comments. I don't think I _should_ merge it, but I > think > I _can_, which makes me uneasy. > > https://invent.kde.org/pim/kcalutils/-/merge_requests/5 had comments, which I > resolved. So, do I merge it? Do I wait for both reviewers to upvote it? > Does a reviewer merge it? Nothing has really changed, you could push to git before in phabricator, you can push to git now in gitlab, it's just a bit easier because the button is there in the web UI. You have the power, as uncle Ben said, use it wisely. If merging something makes you uneasy, probably you shouldnn't be merging it and should ping other people to help review (or convince yourself that it is right and then paying lots of attention to bugs, etc. in case it breaks something). Cheers, albert > > >
Re: Who merges Merge Requests, and when?
On Wednesday, June 10, 2020 12:19:36 PM EDT Glen Ditchfield wrote: > In the Phabricator work flow, I would submit a patch, someone knowledgeable > would accept it, and I would `arc land` it. Clear and simple. > > GitLab doesn't have that "accept" step AFAIK. The KDE Wiki's Infrastructure/ > GitLab page says "Once the Merge Request is accepted, KDE Developers will > merge it for you!" That is imprecise; I'm in the Developer group, but I'm > just a casual. > > So, I have a couple of MRs in flight: > > https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/1 hasn't > attracted any review comments. I don't think I _should_ merge it, but I > think > I _can_, which makes me uneasy. > > https://invent.kde.org/pim/kcalutils/-/merge_requests/5 had comments, which I > resolved. So, do I merge it? Do I wait for both reviewers to upvote it? > Does a reviewer merge it? > Problem: There are not very many people interested in kcalendarcore or kcalutils and even fewer that have the knowledge required to review your MRs. I happen to be 1 of those people. I will look at your MRs soon. I am very happy to have another person interested in these libraries and very much welcome your contributions. In the case of kcalendarcore I had forgotten to setup notifications for that repo. apologies Typically, the maintainer(s) would merge. Neither of these repos have a real maintainer, however. I don't think we have any rules against self-merging though.
Who merges Merge Requests, and when?
In the Phabricator work flow, I would submit a patch, someone knowledgeable would accept it, and I would `arc land` it. Clear and simple. GitLab doesn't have that "accept" step AFAIK. The KDE Wiki's Infrastructure/ GitLab page says "Once the Merge Request is accepted, KDE Developers will merge it for you!" That is imprecise; I'm in the Developer group, but I'm just a casual. So, I have a couple of MRs in flight: https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/1 hasn't attracted any review comments. I don't think I _should_ merge it, but I think I _can_, which makes me uneasy. https://invent.kde.org/pim/kcalutils/-/merge_requests/5 had comments, which I resolved. So, do I merge it? Do I wait for both reviewers to upvote it? Does a reviewer merge it?
