Merged PR at GH, is it possible to reopen and add missing step?

Talk about Documentation
1

There's an error in https://github.com/openwrt/openwrt/pull/21511.

It's missing a saveenv after the 1st setenv command, it makes the initramfs boot with working network, but after you've flashed the sysupgrade image, there's no network anymore, since the new bootcmd wasn't saved.

Searching the forum, people have ran into this.

Can a merged PR be reopened, to fix this ?

1 Like
2

It is usual to open a "bugfix" or "patch" PR and get that merged.
Probably best to open a Github issue and ping toshipp to bring his attention to it.

3

Once merged, the old PR no longer applies and you need to open a new one with the bugfix.

2 Likes
4

that's fine, but will the old "disappear" ?

having two PRs, one correct and the other one incorrect is almost as bad as having only the incorrect one.

sorry if questions seem stupid, I'm not very familiar with GH :frowning:

5

The old PR was merged, so it is now in the code base. It shows up on Github as "closed" and is no longer a Pull Request.

Opening a new PR, once it is merged, will update the code base with the fix.

You could actually make a comment on the old PR, starting with @toshipp to make sure he gets a notification. Explain the issue and see if he answers.

2 Likes
6

I see.

I don't think I've realized which parts actually make it into the code base.

basically you can submit a patch where no code is changed, only comment(s).

7

You can see files or changed files that go into the code base, for example for this merged PR, here:

8

yeah, I probably would have figured it out eventually, if I had looked into the commits a little harder ,)

anyway, I've added a comment to the old PR.

thnx @bluewavenet and @slh.

9

Indeed. When you first start looking at git stuff it all seems like black magic, or at least it did to me.

PRs are not always the clearest to follow either. Then don't get me started on the fact that some people with the access rights to do so, sometimes get sloppy and push directly into master/main instead of opening a PR where it can be reviewed.
Even if they merged their own PR's, at least there would be an easy to follow path/ audit-trail of how things got changed.. But I digress :man_tipping_hand:

1 Like