https://github.com/osquery/osquery logo
Join Slack
Powered by
If someone can please approve the mdfind fix, we c...
# code-review
z
If someone can please approve the mdfind fix, we can start rebasing to get CI passing on PRs: https://github.com/osquery/osquery/pull/8589
@Stefano Bonicatti @alessandrogario @sharvil @seph
s
FWIW I'm good with force merging some of the others if we need to
I think we can probably override CI failkures
But I thumbed it
z
We can -- just need to turn off the setting temporarily
I've just sent comments to everyone asking them to rebase/merge so that their CI will now pass.
If people don't get to it, I might do the override.
s
I couldn't remember if owners could override.
close & reopen might make their branches rebase for CI
s
Sorry for the delay, but you can also close and reopen the PR, that will redo the merge with master, and will pick up things.
z
Oh really??
How does it do that? What if there are conflicts?
s
Just a plain merge afaik.
behind the scene
z
I just closed and reopened this one and I don't see a merge commit added
s
no it's what get passed to the CI
basically what the CI gets it's the merge between the target branch and you PR
z
ohhh
We do that explicitly in our workflows?
s
but the PR itself somehow keeps track of all changes in the target branch, so as soon there's a conflict it will show it, even though the CI doesn't get that version to build
no it's all Github
I mean, ok..
z
I always thought actions/checkout just checked out the commit SHA for the head of the PR branch
s
I think I'm leaving out a big detail
nvm on "the big detail", I was starting to confuse myself in the explanation, anyway:
Copy code
Checking out the ref
  /usr/bin/git checkout --progress --force refs/remotes/pull/8571/merge
  Note: switching to 'refs/remotes/pull/8571/merge'.
  
  You are in 'detached HEAD' state. You can look around, make experimental
  changes and commit them, and you can discard any commits you make in this
  state without impacting any branches by switching back to a branch.
  
  If you want to create a new branch to retain commits you create, you may
  do so (now or later) by using -c with the switch command. Example:
  
    git switch -c <new-branch-name>
  
  Or undo this operation with:
  
    git switch -
  
  Turn off this advice by setting config variable advice.detachedHead to false
  
  HEAD is now at 10337a0d7 Merge 87722e9d69218c12679f534ccdb4e42729a0c802 into 6b87d0cb535af8ecd4d53af39e8921c9b65727bb
z
okay so the checkout action does a merge?
s
it checks out the merge result
z
Thank you for showing me 🙂
s
The weird thing though here is that afaik (or at least what we see experimentally) is that the resulting branch here uses a commit on master that's not updated when master is updated, even though the PR itself knows when a conflicting commit has been merged on master (because the PR shows that the PR cannot be merged)
So I think, it takes a snapshot of whatever commit was on master when the PR was opened and uses that to merge and gives that to the CI, while the PR itself (the UI) keeps track of the latest things on master
But if close and reopen, then it will update the snapshot it took from master. Or obviously if you rebase/merge.
This is also part of the reason why it's possible that although the last CI build on a PR says that everything can be built, or there's no test failing, if something else gets merged before, and the PR has not received any update, then if it gets merged, things could break. It's rare, but it happens.
z
Yeah I've defnitely seen that last case. Thanks for explaining!
Open in Slack
PreviousNext
Powered by