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

Audit icu_list markers #6073

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robertbastian
Copy link
Member

@robertbastian robertbastian commented Feb 5, 2025

I don't think these should be AndV2, OrV2, and UnitV2, in some kind of "list" namespace that is not part of the marker name. @sffc has mentioned he wants to have SymbolsV1 in multiple components, but I think the component name space should be part of the marker. We definitely have instances of using data markers across crates, and this will put us in a situation of a constructor requiring two different SymbolsV1 markers, which will lead to confusing docs and confusing code.

I think the policy should be that the first word of the marker name is the component.

@robertbastian robertbastian requested review from sffc, Manishearth and a team as code owners February 5, 2025 16:32
@robertbastian robertbastian marked this pull request as draft February 5, 2025 16:32
@robertbastian robertbastian changed the title audit icu_list markers Audit icu_list markers Feb 5, 2025
@Manishearth
Copy link
Member

which will lead to confusing docs and confusing code.

This is compelling to me. I think we can be consistent here and even perhaps have the registry check this.

@sffc
Copy link
Member

sffc commented Feb 5, 2025

Writeup of my position on namespaces: #4991 (comment)

@robertbastian robertbastian marked this pull request as ready for review February 7, 2025 10:19
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