[Geany-Devel] Zombified pull requests

Lex Trotman elextr at xxxxx
Wed Jan 6 13:09:19 UTC 2016


>
> The advantage of labels is that you see them on the PR overview. Whereas
> with comments you have to open every single PR and look for strings that may
> indicate a review result. While labels can be named arbitrarily, in the end
> there's going to be a handful common ones which can be looked out for (I
> disagree that free-form comments are clearer than labels for indicating
> actual results).
>
> So comments don't scale, whereas labels could be a real help when looking
> through the list of open PRs for potentially ready PRs.
>

This is true, but I think only those with rights on the repo can use
them, but comments are available to everyone.

[..]
>
> You don't use git head even now with supposedly-perfect merges, so you are
> not lost if it would become a bit less stable :-)

Actually I do ... and its GTK3!!!!!! :)

[...]
>> Yes, better tests for the non-GUI areas would make them easier to
>> change.  Your offer to write the tests is accepted :)
>
>
> Tests are written when 1) a change to the code is prepared and the test
> verifies that change or b) when a bug is fixed and the test verifies the bug
> is truly fixed. Just writing tests out of the blue (I don't intend to make a
> change or fix a bug there) don't work.
>
> However we could start demanding tests as part of PRs (where applicable) and
> perhaps integrate with Travis / other CI.
>
> On the other hand, demanding tests might drive contributors away.

Yeah, its a bit of a no-win, but we should always ask (if the change
is in a non-GUI part).

>
>
>>
>>>> 6. Monster PRs. This goes along with #3. Sometimes PRs are just too big
>>>> to
>>>> be digested by volunteer developers all at once. If we started
>>>> integrating
>>>> such larger changes earlier, and in smaller pieces, I think it would
>>>> increase the likelihood of getting the feature/changes merged into the
>>>> master branch. When a PR is so big, it basically takes as much time to
>>>> fully
>>>> review and test it as it did to make the changes in the first place. I
>>>> personally rarely merge pull requests unless I've reviewed and
>>>> understand
>>>> each line of code that changed, and tested all the cases I can think of.
>>>> As
>>>> volunteer like the rest of us, I just simply can't afford to spend
>>>> days/weeks reviewing, understanding, and testing such large PRs,
>>>> especially
>>>> if I'm rather indifferent on the feature/improvement they implement.
>>>>
>>> Yes, monster PRs are difficult to deal with. But on the other hand I get
>>> the
>>> response that "I don't like to merge this if nothing uses it right now"
>>> (when making a groundwork change required by a later one) when attempting
>>> to
>>> make smaller, incremental changes. Both views are reasonable. Plus,
>>> changes
>>> that merely lay the groundwork and don't have any impact itself seem to
>>> have
>>> very low review priority and are merged with large delay, if at all.
>>>
>>> I also have learned that breaking up large changes into lots of small
>>> commits doesn't help either. It still makes the PR as a whole rather
>>> large
>>> and lots of commits are not handy to deal with either.
>>
>> You are right that individual commits on a monster PR don't help much
>> unless they can be reviewed, tested and committed individually.
>>
>> The way massive changes are handled on Julia is for the OP to put up a
>> PR (or just an issue) with a list of the steps required for the whole
>> change.  Individual PRs that make preparatory changes then refer to
>> this so there is a context for them and they can be smaller and easier
>> to review/test.
>
>
> That's a good idea. IIRC you can even draw checkboxes / todo lists in github
> comments?

Yep, and tick 'em off as they are done.

>
> However there is another catch: I'm hesitent to make PRs on top of other PRs
> (even in the same repository). Heck, I don't know even it's possible. The
> catch is that even if I break large changes into multiple PRs, I can still
> open the PRs only sequientially, so the 2nd PR after the 1st one is complete
> and merged. This makes it hard to see upcoming changes in the context of of
> the early PRs. Is there a solution to this catch?

I'm not aware of a technical solution to that, indeed sometimes if the
OP is impatient there will be PRs that depend on preceding PRs if they
take a while to commit, but at least you can see the dependence in the
overview description and sort of expect it.

>
>
> Best regards.
> _______________________________________________
> Devel mailing list
> Devel at lists.geany.org
> https://lists.geany.org/cgi-bin/mailman/listinfo/devel


More information about the Devel mailing list