Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add uniquify parameter to TemplateDict.add_joined #16213

Closed
wants to merge 1 commit into from
Closed

Add uniquify parameter to TemplateDict.add_joined #16213

wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Sep 3, 2022

The behavior is analogous to that of Args.add_joined.

@fmeum
Copy link
Collaborator Author

fmeum commented Sep 3, 2022

@hvadehra With this change, it would among other things become possible to collect the unique user link flags for a hypothetical cc_static_library in a memory efficient way (see #1920 and in particular @oquenchil's example code). Do you think it's worth the additional API in TemplateDict?

@fmeum fmeum marked this pull request as ready for review September 3, 2022 10:35
@sgowroji sgowroji added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Sep 5, 2022
@fmeum
Add uniquify parameter to TemplateDict.add_joined
40a4ce7 
The behavior is analogous to that of Args.add_joined.

Work towards bazelbuild#1920
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 9, 2022

@hvadehra I rebased onto master, this is ready for review again.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 2, 2022

@oquenchil In case @hvadehra should unavailable, could you perhaps review this PR? It's not something that needs to go into 6.0.

@hvadehra
Copy link
Member

hvadehra commented Nov 2, 2022

So sorry, this dropped off my radar somehow. Reviewing today.

@hvadehra hvadehra assigned hvadehra and unassigned oquenchil Nov 2, 2022
hvadehra
hvadehra approved these changes Nov 2, 2022
View reviewed changes
Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

I think it does make sense to have this, so thanks! Java rules might start using this for jvm_flags as well

@copybara-service copybara-service bot closed this in 9d250ed Nov 2, 2022
@fmeum fmeum deleted the 1920-uniquify branch November 2, 2022 20:04
fmeum added a commit to fmeum/bazel that referenced this pull request Nov 2, 2022
@fmeum
Add uniquify parameter to TemplateDict.add_joined
8764ebb 
The behavior is analogous to that of Args.add_joined.

Closes bazelbuild#16213.

PiperOrigin-RevId: 485571903
Change-Id: Id69de92d703d5202bfc7b50abfbbb4326441f242
@fmeum fmeum mentioned this pull request Nov 2, 2022
Merged
ShreeM01 added a commit that referenced this pull request Nov 2, 2022
@fmeum @ShreeM01
Add uniquify parameter to TemplateDict.add_joined (#16639)
c3259c4 
The behavior is analogous to that of Args.add_joined.

Closes #16213.

PiperOrigin-RevId: 485571903
Change-Id: Id69de92d703d5202bfc7b50abfbbb4326441f242

Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hvadehra hvadehra hvadehra approved these changes

Assignees

@hvadehra hvadehra

Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fmeum @hvadehra @oquenchil @sgowroji