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

Fix #1152: Define OfflineAudioContext constructor with dictionary #1168

Closed

Conversation

rtoy
Copy link
Member

@rtoy rtoy commented Mar 16, 2017

Add new constructor for OfflineAudioContext that takes a required
OfflineAudioContextOptions dictionary to specify the necessary
parameters to create an offline context.

…nary

Add new constructor for OfflineAudioContext that takes a required
OfflineAudioContextOptions dictionary to specify the necessary
parameters to create an offline context.
@padenot padenot self-requested a review March 20, 2017 16:00
@padenot padenot assigned padenot and rtoy and unassigned padenot Mar 20, 2017
Copy link
Member

@padenot padenot left a comment

Choose a reason for hiding this comment

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

lgtm but it needs to be rebased.

@rtoy
Copy link
Member Author

rtoy commented Mar 20, 2017

Thanks @padenot
But I've kind of reconsidered and think OfflineAudioContextOptions should inherit from AudioNodeOptions, as mentioned in #1152 (comment)

@rtoy
Copy link
Member Author

rtoy commented Mar 23, 2017

@padenot Are you working on better defining the existing constructor for OfflineAudioContext? If so, perhaps it's best if I don't do any more work on this until that is done. If not, then perhaps this can include all the needed bits.

The old 3-arg constructor is defined in terms of the new 1-arg
constructor, and the appropriate steps for constructing the context is
given.
@rtoy
Copy link
Member Author

rtoy commented Mar 30, 2017

Closing until @padenot finishes the other in-progress constructor changes.

@rtoy rtoy closed this Mar 30, 2017
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