tools: add update script for googletest #47482
Conversation
|
Review requested:
|
nodejs-github-bot
added
meta
|
Im not sure about updating the gyp file, its too complex to automate it we should probably do it manually |
I tend to agree with that in general. However, in this case, if the script does make a mistake in producing the gyp file, we still have our regular review process as well as Github Actions and Jenkins CI as safeguards, and we can still fix the gyp file manually if necessary. A mistake that is not caught by any of these safeguards could just as well be the result of manual work. In other words, we can take the generated gyp file as a suggestion, which hopefully will be correct in most cases, thus usually removing the need to update it manually. When we or our automated tests notice a problem with the suggested change, we can still resort to updating it manually. I'll try and see what happens with previous versions of googletest (i.e., if the script would have made mistakes in the past). That being said, I'd be happy to switch to a more reliable approach if we can come up with one. |
There was a problem hiding this comment.
I like this approach. Recently, I read about a similar blog post about how Github updates its Ruby version every week by following main branch. We can always revert back or interfere with the upgrade process and I believe this would be a valuable lesson/experience we could benefit as the whole Node.js project.
marco-ippolito
force-pushed
the
deps-googletest-updater
branch
from
4782ef4
to
9304143
Compare
tniessen
force-pushed
the
deps-googletest-updater
branch
from
9304143
to
4c1af7b
Compare
tniessen
added
the
author ready
tniessen
force-pushed
the
deps-googletest-updater
branch
from
4c1af7b
to
a31c52e
Compare
|
Appeased lint-sh. PTAL @marco-ippolito. |
tniessen
force-pushed
the
deps-googletest-updater
branch
from
a31c52e
to
f2eae6b
Compare
|
Rebased again and updated for #47591. |
GoogleTest follows the Abseil Live at Head philosophy, and rarely creates tags or GitHub releases, so instead, follow Google's recommendation and update to the upstream HEAD every once in a while. The tricky bit is properly updating googletest.gyp, and this script might fail doing so in the future. Refs: nodejs/security-wg#828
tniessen
force-pushed
the
deps-googletest-updater
branch
from
f2eae6b
to
635b183
Compare
|
@marco-ippolito Is my understanding correct that the GitHub action will keep updating the same PR instead of opening new ones if a new update is found before the PR is merged? That would be great to reduce noise in this case, if we could just keep the PR open regardless of how often the action runs. |
|
if there is a pr for |
According to https://github.com/gr2m/create-or-update-pull-request-action#how-it-works it should update the existing PR (more specifically, it pushes to the same branch as the previously opened PR for the dependency). I've seen this happen on some of the Undici PRs (e.g. #46502). |
Oh that's great then we are good on this |
|
I like that not creating additional PRs reduces notification noise, but it unfortunately seems to unnecessarily keep old commits and also doesn't update the PR title or description. |
Updating the title/body can be controlled by the |
|
Thanks, I saw that in the docs right after I left the previous comment and opened #47621. Regardless, this PR needs to be re-approved after the last force-push :) |
tniessen
added
the
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 4956271 |
GoogleTest follows the Abseil Live at Head philosophy, and rarely creates tags or GitHub releases, so instead, follow Google's recommendation and update to the upstream HEAD every once in a while. The tricky bit is properly updating googletest.gyp, and this script might fail doing so in the future. Refs: nodejs/security-wg#828 PR-URL: #47482 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
GoogleTest follows the Abseil Live at Head philosophy, and rarely creates tags or GitHub releases, so instead, follow Google's recommendation and update to the upstream HEAD every once in a while. The tricky bit is properly updating googletest.gyp, and this script might fail doing so in the future. Refs: nodejs/security-wg#828 PR-URL: #47482 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
GoogleTest follows the Abseil Live at Head philosophy, and rarely creates tags or GitHub releases, so instead, follow Google's recommendation and update to the upstream HEAD every once in a while. The tricky bit is properly updating googletest.gyp, and this script might fail doing so in the future. Refs: nodejs/security-wg#828 PR-URL: nodejs#47482 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
I haven't done one of these before, so please review carefully.
GoogleTest follows the Abseil Live at Head philosophy, and rarely creates tags or GitHub releases, so instead, follow Google's recommendation and update to the upstream HEAD every once in a while.
The tricky bit is properly updating googletest.gyp, and this script might fail doing so in the future.
Refs: nodejs/security-wg#828