Skip to content

Conversation

FrankYFTang
Copy link
Collaborator

@FrankYFTang FrankYFTang commented Jun 4, 2025

Fix #118

@FrankYFTang
Copy link
Collaborator Author

macOS normail build CI take 2m5s to run, ubuntu take 3m24s to run, checkmemleak take 5m25s to run

@FrankYFTang FrankYFTang requested a review from nciric June 4, 2025 00:37
@FrankYFTang
Copy link
Collaborator Author

@nciric Please review

@FrankYFTang
Copy link
Collaborator Author

Fix #118

@FrankYFTang
Copy link
Collaborator Author

Copy link
Contributor

@nciric nciric left a comment

Choose a reason for hiding this comment

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

I see two ways to go here:

  1. Remove duplication of ICU checkout and some checkout steps (we are potentially paying 2x for LFS checkout) and keep everything in this file
  2. Create a new workflow (e.g. ubuntu-memory-check.yml) and move your valgrind logic there. You will need to copy some of the triggering logic from this file.

@FrankYFTang
Copy link
Collaborator Author

@FrankYFTang
Copy link
Collaborator Author

I see two ways to go here:

  1. Remove duplication of ICU checkout and some checkout steps (we are potentially paying 2x for LFS checkout) and keep everything in this file
  2. Create a new workflow (e.g. ubuntu-memory-check.yml) and move your valgrind logic there. You will need to copy some of the triggering logic from this file.

I address both issues . PTAL

@FrankYFTang FrankYFTang merged commit 9dd5bcc into unicode-org:main Jun 5, 2025
1 check passed
@FrankYFTang FrankYFTang deleted the memleakcheck branch June 5, 2025 21:33
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.

Add memory leak checking to pull request checks
2 participants