Skip to content

Implement NameIDMapper as a module to use for translating names to IDs - #5

Merged
yushan8 merged 3 commits into
mainfrom
diff-between-graph
Jan 30, 2026
Merged

yushan8 merged 3 commits into
mainfrom
diff-between-graph

Conversation

@yushan8

@yushan8 yushan8 commented Jan 28, 2026 •

Copy link
Copy Markdown
Contributor

Translation to OptimizedTargets from name -> id is common functionality. This module is needed for comparing target graphs and converting Results -> OptimizedTargets to store the target graph. A separate module NameIDMapper is needed to assign names to ID numbers.

Tested with unit test

Update

Update test

Update
@yushan8
yushan8 force-pushed the diff-between-graph branch from 5d1aa01 to 5b99f71 Compare January 29, 2026 22:55
@yushan8 yushan8 changed the title Implement comparison between two target graphs Implement NameIDMapper as a module to use for translating names to IDs Jan 29, 2026
@yushan8
yushan8 marked this pull request as ready for review January 29, 2026 22:58
@yushan8
yushan8 requested review from a team as code owners January 29, 2026 22:58

@behinddwalls behinddwalls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ship It!

Comment thread core/targethasher/graph.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we have this translation function in another file? To avoid conflicts when we sync Uber targethasher to this file,

@yushan8 yushan8 Jan 30, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point I'll move it

@yushan8
yushan8 merged commit 1991c27 into main Jan 30, 2026
1 check passed
@yushan8
yushan8 deleted the diff-between-graph branch February 4, 2026 22:20
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