Skip to content

Conversation

@jbachorik
Copy link
Collaborator

The 'fetch/checkout' combo is not working well for updating the upstream repo.
It is replaced with 'clean/clone/checkout' instead

@jbachorik jbachorik requested a review from zhengyu123 July 23, 2025 23:26
@github-actions
Copy link

🔧 Report generated by pr-comment-cppcheck

CppCheck Report

Errors (2)

Warnings (8)

Style Violations (300)

@github-actions
Copy link

🔧 Report generated by pr-comment-scanbuild

commandLine 'rm', '-rf', targetDir.absolutePath
}
exec {
commandLine 'git', 'clone', '--branch', branch_lock, 'https://github.com/datadog/async-profiler.git', targetDir.absolutePath
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove the depth 1 ?

exec {
  commandLine 'git', 'clone', '--depth', '1', '--branch', branch_lock, 'https://github.com/datadog/async-profiler.git', targetDir.absolutePath
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does not allow moving the branch head and locking on a certain commit. It basically defeats the purpose of having the commit hash in the lock.
The async-profiler history is pretty light-weight so we are not saving that much by having just 1 level.
We may revisit this later and maybe lock the branch only and use the commit just as a check for whether we need to refresh the contents?

Copy link
Contributor

@r1viollet r1viollet left a comment

Choose a reason for hiding this comment

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

minor comment otherwise LGTM

@jbachorik jbachorik merged commit b2e06ca into main Jul 24, 2025
184 checks passed
@jbachorik jbachorik deleted the jb/reset_ap_cache branch July 24, 2025 09:08
@github-actions github-actions bot added this to the 1.29.0 milestone Jul 24, 2025
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.

3 participants