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

collector: Refresh example in doccomment #3292

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Conversation

inahga
Copy link
Contributor

@inahga inahga commented Jul 10, 2024

Give a little bit of love to this doccomment. Suggest use of the PrivateCollectorCredential instead of bringing in the unstable janus_core. Also provide an example of using a fixed size query.

@inahga inahga requested a review from a team as a code owner July 10, 2024 16:25
Copy link
Contributor

@divergentdave divergentdave left a comment

Choose a reason for hiding this comment

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

LGTM

FWIW janus_core is published on crates.io, while janus_aggregator_core is not. (it is a dependency of janus_collector as well)

//! HpkeKdfId::HkdfSha256,
//! HpkeAeadId::Aes128Gcm,
//! ).unwrap();
//! let task_id = TaskId::from_str("[your DAP task ID here]").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend leaving this un-indented, since the function's opening and closing braces won't appear in the final output. (It's just present to allow us to use .await in this context)

@inahga
Copy link
Contributor Author

inahga commented Jul 10, 2024

FWIW janus_core is published on crates.io, while janus_aggregator_core is not.

Indeed, but a while ago we decided ago to say "please don't use janus_core directly", so that we can make breaking changes to it. Granted, I don't think we really landed on hard consensus on it, so we can rehash the discussion if we need. #2390 (comment) https://github.com/divviup/janus?tab=readme-ov-file#versioning-and-stability

@inahga inahga merged commit f8a227f into main Jul 10, 2024
8 checks passed
@inahga inahga deleted the inahga/fix-collector-doc branch July 10, 2024 22:38
inahga added a commit that referenced this pull request Jul 11, 2024
* collector: Refresh example in doccomment

* Remove unnecessary indentation
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.

None yet

2 participants