<div dir="ltr"><div dir="ltr">On Thu, Aug 25, 2022 at 2:16 AM Harald Sitter <<a href="mailto:sitter@kde.org">sitter@kde.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Wed, Aug 24, 2022 at 3:31 PM Noah Davis <<a href="mailto:noahadvs@gmail.com" target="_blank">noahadvs@gmail.com</a>> wrote:<br>
><br>
> A week ago, I ran into an unexpected issue while working on a QML port<br>
> of Spectacle. There is an undocumented pre-receive hook that sets a<br>
> 100 commit limit for all branches of official repos, including work<br>
> branches. The error message it gave me told me to file a sysadmin<br>
> ticket, so I did that.<br>
><br>
> The sysadmin ticket link: <a href="https://phabricator.kde.org/T15683" rel="noreferrer" target="_blank">https://phabricator.kde.org/T15683</a><br>
><br>
> I don't think I can make the ticket public, so here's the conversation:<br>
><br>
> --- Start of conversation<br>
><br>
> ndavis (me):<br>
> For normal branches, I would understand this since there were issues<br>
> with spamming #kde-devel in the past. This isn't necessary for work<br>
> branches, right? I thought the point of the work/ naming convention<br>
> was to prevent this issue.<br>
><br>
> bcooksley:<br>
> Work branches weren't meant to be used for large feature developments<br>
> with 100+ commits in them, which is why this is being blocked.<br>
> In it's current state - even if rebased - the branch will not be able<br>
> to be merged without Sysadmin intervention.<br>
> Will this be squashed prior to being merged?<br>
><br>
> ndavis:<br>
> > Will this be squashed prior to being merged?<br>
><br>
> Yes<br>
><br>
> > Work branches weren't meant to be used for large feature developments with 100+ commits in them, which is why this is being blocked.<br>
><br>
> Is this documented anywhere? I don't understand why work branches<br>
> would block this. The git message says notifications are the reason<br>
> why the push was blocked, but I thought work branches didn't send<br>
> notifications?<br>
><br>
> bcooksley:<br>
> The message is a little misleading, but that is mostly because this<br>
> situation isn't supposed to really occur. You are correct that work<br>
> branches don't send notifications.<br>
> As such it is not documented anywhere.<br>
> From a policy perspective the reason why we don't allow work branches<br>
> to grow beyond 100 commits is because if we did then you would never<br>
> be able to merge them in - not without squashing anyway.<br>
> This therefore makes you "squash as you go".<br>
> I would generally recommend that major features be developed in an<br>
> ordinary branch to allow those that monitor kde-commits and other<br>
> commit feeds to chime in with real-time reviews, and then merged using<br>
> a traditional Git merge (vs. our more typical rebase)<br>
><br>
> ndavis:<br>
> > The message is a little misleading, but that is mostly because this situation isn't supposed to really occur. You are correct that work branches don't send notifications. As such it is not documented anywhere.<br>
><br>
> If we are going to keep this limitation, we should document it so that<br>
> nobody else makes the same mistake as me. We can't assume that<br>
> something that isn't supposed to happen won't happen because there's<br>
> no reason to assume this limitation would exist.<br>
><br>
> > From a policy perspective the reason why we don't allow work branches to grow beyond 100 commits is because if we did then you would never be able to merge them in - not without squashing anyway.<br>
> This therefore makes you "squash as you go".<br>
><br>
> I don't understand why we have this policy. If we can't merge an MR<br>
> with over 100 commits, why can't we just tell the person making an MR<br>
> when they post the MR to squash it? I think it would make more sense<br>
> for this policy to apply only when pushing to master or version<br>
> branches.<br>
><br>
> > I would generally recommend that major features be developed in an ordinary branch to allow those that monitor kde-commits and other commit feeds to chime in with real-time reviews,<br>
><br>
> In my experience, nobody does that. Nobody has time to review unless<br>
> you explicitly request help or you post an MR.<br>
> I don't know the protocol for discussing these kinds of things, so let<br>
> me know if this discussion should be moved elsewhere.<br>
><br>
> --- End of conversation<br>
><br>
> After this, I decided it would be better to discuss this with the<br>
> broader KDE community and I closed the ticket.<br>
><br>
> I did try to switch to a normal branch as Ben Cooksley suggested, but<br>
> that had the same limitation. I have not tested using a fork, but some<br>
> of the people I've talked to casually (I can't remember who) seemed to<br>
> be saying that fork branches don't have this limitation, which I think<br>
> would make the limit on work branches kind of pointless if it's true.<br>
><br>
> I understand the concern about making unmergeable merge requests, but<br>
> I don't think a hard 100 commit limit pre-recieve hook is the right<br>
> way to deal with that. That's something that should be handled by<br>
> reviewers, not automated systems, because sometimes info needs to be<br>
> kept until it is time to merge. Instead of having lots of small<br>
> commits to keep track of each change as much as possible until it's<br>
> time to review the MR, I've had to squash them into mega commits with<br>
> lines in the commit message for each commit that got squashed.<br>
> Normally, I would squash at the end of the review process to ensure<br>
> that all relevant info is kept and irrelevant info is discarded.<br>
><br>
> What does the broader KDE development community think? Should there be<br>
> a commit limit and how large should it be? Only KDE devs can make work<br>
> branches, so the pool of people who can make branches with large<br>
> amounts of commits is already fairly limited.<br>
<br>
Yeah, I don't see why there needs to be a limit at all on work<br>
branches, let alone one that low.<br></blockquote><div><br></div><div>The limitation is aligned with the maximum number of new commits you are allowed to introduce to a standard branch.</div><div>We have those limits because the commit hooks carry out processing on a per commit basis for all new commits introduced to standard branches - and are there to protect all the other systems downstream from Gitlab.</div><div><br></div><div>The reason behind applying those same limits to work branches is because there is an expectation that you would be able to merge a work branch into a standard branch at the conclusion of the work.</div><div>You won't be able to do that if more than 100 new commits are in that work branch.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
HS<br></blockquote><div><br></div><div>Regards,</div><div>Ben </div></div></div>