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.

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

Rename PrivateAttribution.aggregators to aggregationServices #67

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

apasel422
Copy link
Contributor

To match the surrounding prose, and for consistency with the names of the corresponding IDL types.

Fixes #66

@apasel422 apasel422 marked this pull request as ready for review January 14, 2025 13:43
@@ -603,7 +603,7 @@ The arguments to <a method for=PrivateAttribution>measureConversion()</a> are as
<dt><dfn>aggregator</dfn></dt>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unclear to me if we want to change the terminology here as well.

@andyleiserson
Copy link
Collaborator

I think @martinthomson had made this change in the other direction (aggregationServices -> aggregators). We definitely need to resolve the inconsistency -- but maybe the less verbose name is preferable?

@apasel422
Copy link
Contributor Author

We definitely need to resolve the inconsistency -- but maybe the less verbose name is preferable?

I don't have a strong feeling either way, but agree on resolving the inconsistency.

@martinthomson
Copy link
Collaborator

Let's resolve that question in discussion on the issue (which I'll put on the agenda for our next meeting).

To match the surrounding prose, and for consistency with the names of
the corresponding IDL types.

Fixes patcg#66
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.

Inconsistent IDL naming: aggregationServices vs aggregators
3 participants