Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check submodule forward progress #8949

Merged
merged 5 commits into from
Sep 22, 2024
Merged

Conversation

hlinnaka
Copy link
Contributor

@hlinnaka hlinnaka commented Sep 6, 2024

We frequently mess up our submodule references. This adds one safeguard: it checks that the submodule references point to the corresponding REL_*_STABLE_neon branches, or to some commit descending from them.

As next step, I'm thinking that we should automate things so that when you merge a PR to the 'neon' repository that updates the submodule references, the REL_*_STABLE_neon branches are automatically updated to match the submodule references. That way, you never need to manually merge PRs in the postgres repository, it's all triggered from commits in the 'neon' repository. But that's not included here.

@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch 8 times, most recently from b393d76 to e54ce82 Compare September 6, 2024 17:11
@hlinnaka hlinnaka requested review from bayandin, a team and lubennikovaav and removed request for a team September 6, 2024 17:14
@hlinnaka
Copy link
Contributor Author

hlinnaka commented Sep 6, 2024

Note: This PR contains a commit that causes the new test to fail on purpose on the v15 submodule, so that you can see what the failure looks like.

edit: removed that commit

@hlinnaka hlinnaka requested a review from ololobus September 6, 2024 17:15
Copy link

github-actions bot commented Sep 6, 2024

5055 tests run: 4891 passed, 0 failed, 164 skipped (full report)


Flaky tests (16)

Postgres 17

Postgres 16

Postgres 15

Code coverage* (full report)

  • functions: 31.9% (7435 of 23317 functions)
  • lines: 49.9% (59920 of 120118 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
c4dfdf8 at 2024-09-21T19:11:40.277Z :recycle:

@hlinnaka hlinnaka marked this pull request as ready for review September 7, 2024 11:35
@hlinnaka hlinnaka requested a review from a team as a code owner September 7, 2024 11:35
.github/workflows/build_and_test.yml Show resolved Hide resolved
.github/workflows/build_and_test.yml Outdated Show resolved Hide resolved
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch from e54ce82 to 7fda256 Compare September 11, 2024 10:58
Copy link

@orca-security-us orca-security-us bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch from 7fda256 to b9e6386 Compare September 11, 2024 22:05
hlinnaka added a commit that referenced this pull request Sep 11, 2024
We frequently mess up our submodule references. This adds one
safeguard: it checks that the submodule references point to the
corresponding REL_*_STABLE_neon branches, or to some commit descending
from them.

As next step, I'm thinking that we should automate things so that when
you merge a PR to the 'neon' repository that updates the submodule
references, the REL_*_STABLE_neon branches are automatically updated
to match the submodule references. That way, you never need to
manually merge PRs in the postgres repository, it's all triggered from
commits in the 'neon' repository. But that's not included here.
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch from b9e6386 to 0190948 Compare September 11, 2024 22:49
hlinnaka added a commit that referenced this pull request Sep 16, 2024
We frequently mess up our submodule references. This adds one
safeguard: it checks that the submodule references point to the
corresponding REL_*_STABLE_neon branches, or to some commit descending
from them.

As next step, I'm thinking that we should automate things so that when
you merge a PR to the 'neon' repository that updates the submodule
references, the REL_*_STABLE_neon branches are automatically updated
to match the submodule references. That way, you never need to
manually merge PRs in the postgres repository, it's all triggered from
commits in the 'neon' repository. But that's not included here.
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch 5 times, most recently from 24dd191 to 7cedf5d Compare September 16, 2024 20:12
@hlinnaka
Copy link
Contributor Author

I just found out that a similar action already exists: https://github.com/jtmullen/submodule-branch-check-action. I did not find that earlier. I'll investigate using that instead..

@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch 5 times, most recently from aed5647 to 782e262 Compare September 16, 2024 22:40
hlinnaka added a commit that referenced this pull request Sep 20, 2024
We frequently mess up our submodule references. This adds one
safeguard: it checks that the submodule references point to the
corresponding REL_*_STABLE_neon branches, or to some commit descending
from them.

As next step, I'm thinking that we should automate things so that when
you merge a PR to the 'neon' repository that updates the submodule
references, the REL_*_STABLE_neon branches are automatically updated
to match the submodule references. That way, you never need to
manually merge PRs in the postgres repository, it's all triggered from
commits in the 'neon' repository. But that's not included here.
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch 2 times, most recently from 4e4a6a4 to 67efddc Compare September 20, 2024 10:33
hlinnaka added a commit that referenced this pull request Sep 20, 2024
We frequently mess up our submodule references. This adds one
safeguard: it checks that the submodule references point to the
corresponding REL_*_STABLE_neon branches, or to some commit descending
from them.

As next step, I'm thinking that we should automate things so that when
you merge a PR to the 'neon' repository that updates the submodule
references, the REL_*_STABLE_neon branches are automatically updated
to match the submodule references. That way, you never need to
manually merge PRs in the postgres repository, it's all triggered from
commits in the 'neon' repository. But that's not included here.
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch from bc53f6b to b131ddf Compare September 20, 2024 16:10
We frequently mess up our submodule references. This adds one
safeguard: it checks that the submodule references point to the
corresponding REL_*_STABLE_neon branches, or to some commit descending
from them.

As next step, I'm thinking that we should automate things so that when
you merge a PR to the 'neon' repository that updates the submodule
references, the REL_*_STABLE_neon branches are automatically updated
to match the submodule references. That way, you never need to
manually merge PRs in the postgres repository, it's all triggered from
commits in the 'neon' repository. But that's not included here.
Because the checks are fairly expensive, taking about 4 minutes.
…anches

The checks were too strict. We want to allow forward progress. The
idea is that you merge the main neon repo PR first, and then update
the REL_*_STABLE_neon branches, not the other way round.
@hlinnaka hlinnaka force-pushed the check-submodule-forward-progress branch from b131ddf to c4dfdf8 Compare September 21, 2024 01:05
@hlinnaka hlinnaka merged commit c9b2ec9 into main Sep 22, 2024
83 checks passed
@hlinnaka hlinnaka deleted the check-submodule-forward-progress branch September 22, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants