Newbie mistakes with PR's

[Edward K. Ream]

For the last few days I have been struggling with making good PR's for Félix's leoInteg project. Now I see the newbie mistakes I made, and how to correct them. Here's a summary:

Seeing diffs before creating the PR

I use large fonts, so at first I didn't see that when I click the "New PR" button all the diffs appear below the text area describing the PR. So now I can check that the diffs are what I expect before creating the PR. Once the PR is created I have not found a good way to revise it, so this little Aha is actually very important.

Base all feature branches on dev

Previously, I based my feature branches on an "ekr" branch. But this complicates everything. Far better to base all feature branches on dev  (or devel in Leo).

Create a new feature branch for all changes

PR's should do exactly one thing. This makes it easier for the review to study and accept or reject PR's. As a result, it's best to create a new feature branch for every separate code change, no matter how small.

Summary

PR's are much easier to manage when I follow these guidelines.

Edward

[Thomas Passin]

On Friday, July 24, 2020 at 8:00:16 AM UTC-4, Edward K. Ream wrote:

Base all feature branches on dev

Previously, I based my feature branches on an "ekr" branch. But this complicates everything. Far better to base all feature branches on dev  (or devel in Leo).

And remember to update your branch to its parent before creating the PR.  You don't want to be accidentally including recent parent's changes into your PR.

 

[Félix]

Indeed Thomas, good point,

pulling modifs from the branch you started your branch from, because the original author of the source branch updated it somewhat, will remove needs to 'fix merge conflicts' if any later on.

[Edward K. Ream]

On Fri, Jul 24, 2020 at 7:10 AM Thomas Passin <tbp1...@gmail.com> wrote:

> And remember to update your branch to its parent before creating the PR.  You don't want to be accidentally including recent parent's changes into your PR.

Exactly. Happily, now that I know where to find the diffs before creating the PR, I can catch such "lags" before creating the PR.

Edward

[Edward K. Ream]

On Fri, Jul 24, 2020 at 9:00 AM Félix <felix...@gmail.com> wrote:

Indeed Thomas, good point,

pulling modifs from the branch you started your branch from, because the original author of the source branch updated it somewhat, will remove needs to 'fix merge conflicts' if any later on.

Good to know. Thanks.

Edward

[Edward K. Ream]

On Friday, July 24, 2020 at 7:00:16 AM UTC-5, Edward K. Ream wrote:

> [Here is a summary of newbie mistakes with PRs]

I just made another mistake. I merged the keys branch into devel without a PR. That's too bad. The PR would have summarized all the changed.

gitk shows branch info, so all is not lost. However, a PR would have been simpler. Getting git log to report what I want hasn't been easy.

I have just added my hand-written notes of the project to #1269. That's a high-level summary, but the diffs in the PR would also been useful.

In short, I'm not likely ever to merge a long-lived branch into devel without a PR.

Edward

[Edward K. Ream]

On Saturday, July 25, 2020 at 10:32:23 AM UTC-5, Edward K. Ream wrote:

> I just made another mistake. I merged the keys branch into devel without a PR. That's too bad. The PR would have summarized all the changed.

I used gitk to guess at the first relevant commit.

Here is a Leo script that shows the diffs between the first relevant commit and the latest commit.

import leo.commands.editFileCommands as efc

efc.GitDiffController(c).diff_two_revs(
     rev1='4b8320485c0', # first keys commit
     rev2='2507a1fcc70', # Latest devel commit
     directory=None,
)

Many of the changes shown are irrelevant and would not have appeared in the PR.

None of this would have been necessary had I issued a PR.

Edward