Skip to content

Add CI for valgrind fix issues/118#121

Merged
FrankYFTang merged 1 commit intounicode-org:mainfrom
FrankYFTang:memleakcheck
Jun 5, 2025
Merged

Add CI for valgrind fix issues/118#121
FrankYFTang merged 1 commit intounicode-org:mainfrom
FrankYFTang:memleakcheck

Conversation

@FrankYFTang
Copy link
Copy Markdown
Collaborator

@FrankYFTang FrankYFTang commented Jun 4, 2025

Fix #118

@FrankYFTang
Copy link
Copy Markdown
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
Copy Markdown
Collaborator Author

@nciric Please review

@FrankYFTang
Copy link
Copy Markdown
Collaborator Author

Fix #118

@FrankYFTang
Copy link
Copy Markdown
Collaborator Author

Copy link
Copy Markdown
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
Copy Markdown
Collaborator Author

@FrankYFTang
Copy link
Copy Markdown
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