Repository navigation
allow to fail atime job in GHCI / use merge commits to allow branch deletion #7363
Description
Activity
I'm not sure this is a good idea.
If the atime job is failing there is probably a reason that needs fixing. for example I see this PR https://github.com/Rdatatable/data.table/actions/runs/18464808691/job/52604142621?pr=7361 has atime error below,Error in value[[3L]](cond) : Error in revparse_single(object, branch): Error in 'git2r_revparse_single': Requested object could not be found when trying to checkout ffe431fbc1fe2d52ed9499f78e7e16eae4d71a93 Calls: <Anonymous> ... tryCatch -> tryCatchList -> tryCatchOne -> <Anonymous> Timing stopped at: 16.42 3.21 17.99 Execution haltedthis means that somebody deleted a branch with a commit required in an atime test.
Searching for "ffe43" in https://github.com/Rdatatable/data.table/blob/master/.ci/atime/tests.R#L47 shows this lineFast = "ffe431fbc1fe2d52ed9499f78e7e16eae4d71a93" # Last commit of the PR (https://github.com/Rdatatable/data.table/pull/4386/commits) where the performance was improved.
which has a reference to the PR #4386 that had the deleted branch. The fix which I just did was going to that PR page and clicking "Restore branch" button at the bottom.
That's a fair point, how should we know if a branch is safe to delete?
I have been trying to use the
atimelabel to mark branches as required for the GHAOk, does that mean we should keep those branches forever? This feels suboptimal. I know many users who do git fetch origin rather than specific branch and it pollutes their git checkout experience... That why I cleaned up old merged branches recently.
yes with the current setup we have to keep old branches with atime commits forever.
Reacted by Toby Dylan HockingI agree it would be better but I think the benefit of {atime} outweighs this cost for now. Open to ideas for how else to make it work.
Instead of pointing to a commit from a branch atime could point to a merge commit. That will be always available in master.
That seems to be the optimal way.Alternatively we could have a fork that keeps all branches needed for atime.
Reacted by Benjamin Schwendingerusing merge commits sounds like a good idea to me.
My memory is that doesn't work somehow, but I could be wrong, please test it out.
- changed the title
[-]allow to fail atime job in GHCI[/-][+]allow to fail atime job in GHCI / use merge commits to allow branch deletion[/+]on Oct 16, 2025 I guess the downside of this approach is the atime test is always added in a separate PR, which requires splitting it off before merge:
- file PR including performance regression test
- upon PR approval, split the atime portion off to its own PR, revert in the current PR
it would be nice to keep the performance test tightly coupled with the code it's intended for, but I think on balance the current approach is preferred to keeping the old branches around indefinitely.
The third option we have, is to squash manually, but this seems more error prone :/
Apologies for the late input (underwent a surgery at the time of pings), but I too am of the opinion that keeping branches just for performance checks on PR would be redundant, so using merge commits sounds great to me. I mentioned it a few times before to Toby about it being the solution (and here too for e.g.)
Reacted by Jan Gorecki
You can try using the
continue-on-errordirective at the job level. Maybe base it off labels (then apply that label to this PR) since usingpull_request.numberwould be redundant; e.g. say we name the label 'ignore-atime-failure':Originally posted by @Anirban166 in #7361 (comment)