Skip to content

Fix compile error due to missing constant#10920

Merged
hashhar merged 1 commit intotrinodb:masterfrom
martint:power-of-ten
Feb 3, 2022
Merged

Fix compile error due to missing constant#10920
hashhar merged 1 commit intotrinodb:masterfrom
martint:power-of-ten

Conversation

@martint
Copy link
Copy Markdown
Member

@martint martint commented Feb 3, 2022

The constant was hidden in a recent change and caused
a logical merge error that resulted in build failures.

@cla-bot cla-bot bot added the cla-signed label Feb 3, 2022
The constant was hidden in a recent change and caused
a logical merge error that resulted in build failures.
@hashhar hashhar merged commit 12714bf into trinodb:master Feb 3, 2022
@github-actions github-actions bot added this to the 370 milestone Feb 3, 2022
@findepi
Copy link
Copy Markdown
Member

findepi commented Feb 3, 2022

The constant was hidden in a recent change and caused

#10404 was merged after 41 days after it was tested on the CI.

@nineinchnick would it be possible to invalidate CI results after some time?

@nineinchnick
Copy link
Copy Markdown
Member

The constant was hidden in a recent change and caused

#10404 was merged after 41 days after it was tested on the CI.

@nineinchnick would it be possible to invalidate CI results after some time?

We would need a bot to rerun pipelines after some time.

But a better solution would be to verify every PR against the master (rebased) before merging. There's some support for this in Github: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/using-a-merge-queue

The merge queue creates temporary preparatory branches to validate pull requests against the latest version of the base branch. To ensure that GitHub validates these preparatory branches, you may need to update your CI configuration to trigger builds on branch names starting with gh-readonly-queue/{base_branch}.

@hashhar
Copy link
Copy Markdown
Member

hashhar commented Feb 3, 2022

I've seen in other projects using https://github.com/actions/stale to mark PR as stale from n days since last push.
And having a job in GHA which gets trigerred on label addition (not on removal) which marks itself failed if the stale label is present, hence making CI appear red.

It's advisory only though.

An alternative is to make it easier to rebase. maybe something like commenting /rebase should trigger rebase + add a "needs rebase" label if PR got stale.

We would need a bot to rerun pipelines after some time.

Re-run re-runs on same merge commit - not a fresh one. You need to perform rebase or push commits.

@findepi
Copy link
Copy Markdown
Member

findepi commented Feb 3, 2022

We would need a bot to rerun pipelines after some time.

@nineinchnick i meant marker, or invalidation. Didn't mean a re-run.

An alternative is to make it easier to rebase. maybe something like commenting /rebase should trigger rebase + add a "needs rebase" label if PR got stale.

rebase is not needed, and obliterates pr history

Re-run re-runs on same merge commit - not a fresh one. You need to perform rebase or push commits.

that's a trap. thanks @hashhar for pointing this out

So if we have advisory that detects stale PRs, we need to also ensure that people don't fall into this trap.
E.g. a CI job could inspect the merge commit and fail if it's old.

I like @hashhar idea of a check that's separate from CI job itself.
Like the cla bot works: doesn't take resources, and red is still red.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants