I see a new Classify Changes CI step, which seems ...
# development
a
I see a new Classify Changes CI step, which seems to be consistently failing for me: https://github.com/pantsbuild/pants/actions/runs/3706006467/jobs/6280703870
Is there a way around this or is it just broken for PRs that pre-date this step?
e
You might edit this value on your branch?: https://github.com/pantsbuild/pants/actions/runs/3706006467/workflow#L25 Make it higher.
a
@enough-analyst-54434 at which point it would get merged into
main
, right?
or is that just for a commit or three?
e
At which point it would get checked in to main.
If it's a problem for you it will be a problem for someone else.
Its a speed hack.
a
Fine 🙂
e
I did not write the thing, but ~clearly that must be the only reason not to default to 0 (all)
Speed hacks seem to permeate our lives.
h
@ancient-vegetable-10556 you can also squash your 14 commits in that PR
Yes, it is a speed hack
but upping that number to like 100 will not have a noticeable impact
a
OK, I’ll set it to 100 then
h
Maybe add a comment that this is a speed hack but we pick a number larger than the number of commits expected in any PR
a
(there’ll be big branches again, I’m sure)
Done
@happy-kitchen-89482 that has not worked
It appears to only work with 0 or 2?
(back in 15-30)
e
Ah, weird. That action does its own git-fu even though there was a prior checkout action. Very odd. Do both actions need to be steps there?
h
huh, you'd think 10 would include 2, so I must be misunderstanding something
The action's own example (https://github.com/tj-actions/changed-files) shows that you need to do your own checkout first
with 0 or 2
a
aha
Let’s see if we can fix that then
h
Looking into what exactly the deal is, i.e., why specifically 0 or 2
a
several attempts aren’t working 😕
h
even setting to 2?
e
Be clear about in which action.
a
Yeah, just blew it away, but let me try again
e
It is confusing!
They both take a depth.
a
2022-12-15 at 9.14 AM.png
this is what I am about to try
e
Yeah, wrong one
a
I tried the other one
e
Its not checkout that needs 2 its the next action
CONFUSAING
h
Well, on the changed-files action the description of the fetch-depth input is "Initial depth of the branch history fetched"
which I think means it needs to match whatever was passed to the checkout action?
a
the docs say to set it on
checkout
e
This is all just to get a list of changed files? Maybe just replace the action after checkout with git commands?
a
the docs say: • checkout with depth 0 or 2 • then run
get-changed-files
.
trying with a value of 2
h
The docs say stuff like:
Copy code
- IMPORTANT: For push events you need to include fetch-depth: 0 OR fetch-depth: 2 depending on your use case.
- For monorepos where pulling all the branch history might not be desired, you can omit fetch-depth for pull_request events.
But it's unclear whether they mean on checkout or on changed-files
e
2 means nothing to checkout, I'll say that - just 0 and N
a
except that it just worked
h
Oh, btw, also try upgrading changed-files to v34.6.1, which it looks like has more helpful error messages in this regard
a
with 2
e
Insane
h
So weird
you would think "fetch depth 10" would include everything that "fetch depth 2" fetches, and a bit more
a
I bet the action doesn’t actually walk the SHA tree and instead looks at the earliest and latest commits available locally
h
Wait, which fetch depth did you change?
a
the one in
classify_changes
h
ah, which we don't set at all today?
we do, to 10
h
That is on checkout
a
right
that’s the thing that does the fetch
and then the custom action acts on the git repo that was fetched
and I bet it’s doing extremely dubious things instead of using the git api
h
Yes, but there is also a fetch-depth argument on the
tj-actions/changed-files
action
e
The diff though says it all: crazy-town: https://github.com/pantsbuild/pants/pull/17716/files#
h
What each of them means is what we are discussing here
e
TIL 2 > 10
h
Yeah, so I am trying to figure out what this all means, rather than groping in the dark
e
I'd drop the action!
Instead of figuring out his crazy semantics