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

Make the timers crate follow submodule design #36

Closed
4 tasks
fitzgen opened this issue Mar 20, 2019 · 5 comments
Closed
4 tasks

Make the timers crate follow submodule design #36

fitzgen opened this issue Mar 20, 2019 · 5 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@fitzgen
Copy link
Member

fitzgen commented Mar 20, 2019

The crates/timers crate should follow the submodule design that we came up with in #27 and are formalizing in #34.

That is:

  • Move the callbacks-based APIs into a callbacks submodule
  • Move the futures-based APIs into a futures module
    • Make the cargo dependency on the futures crate optional
    • Only expose the futures submodule when the futures feature is enabled
@fitzgen fitzgen added bug Something isn't working good first issue Good for newcomers labels Mar 20, 2019
@Pauan
Copy link
Contributor

Pauan commented Mar 20, 2019

I don't like having a feature which has the same name as a package dependency (i.e. the futures crate), so I think we need to change the feature name.

@OddCoincidence
Copy link
Contributor

Wouldn't the full name be gloo-futures though? This seems fine / non-ambiguous to me.

@Pauan
Copy link
Contributor

Pauan commented Mar 21, 2019

No, this proposal says that the feature will be called futures (which conflicts with the futures dependency).

It's unusual to prefix a feature like that, but I'm not against gloo-futures, as you say it is unambiguous.

@OddCoincidence
Copy link
Contributor

Oh sorry, I misread your original comment (assumed you were referring to the crate name). Isn't having a feature named the same as package dependency relatively common? e.g. I see features called serde quite often.

Aehmlo added a commit to Aehmlo/gloo that referenced this issue Mar 22, 2019
The future/stream versions are behind the "futures" feature gate.
This gate is propagated through the workspace using appropriate feature
names.

Closes rustwasm#36.
@Pauan
Copy link
Contributor

Pauan commented Mar 23, 2019

@OddCoincidence I was referring to both the crate name and the feature name (since they're currently the same). Sorry I wasn't clear enough!

It may be common, but it's also wrong (or at least heavily problematic): #38 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants