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

Oscillator node message #242

Merged
merged 2 commits into from Apr 30, 2019
Merged

Oscillator node message #242

merged 2 commits into from Apr 30, 2019

Conversation

@PurpleHairEngineer
Copy link
Contributor

PurpleHairEngineer commented Apr 27, 2019

Implemented an audio node message that is specific to OscillatorNode (using BiquadFilterNode as a model) which updates the node's oscillator type.

Update josephhutch/media fork to reflect servo/media flat directory structure
@PurpleHairEngineer
Copy link
Contributor Author

PurpleHairEngineer commented Apr 27, 2019

I forgot to commit another relevant file. My bad.

@ferjm
ferjm approved these changes Apr 29, 2019
Copy link
Member

ferjm left a comment

r=me with the comments applied, please.

Thanks!

audio/oscillator_node.rs Outdated Show resolved Hide resolved
audio/oscillator_node.rs Outdated Show resolved Hide resolved
@ferjm
Copy link
Member

ferjm commented Apr 29, 2019

It would also be good to squash the commits before merging. Thanks

@PurpleHairEngineer
Copy link
Contributor Author

PurpleHairEngineer commented Apr 29, 2019

@ferjm I made the changes you requested. However, I am not sure how to squash the commits before merging?

@ferjm
Copy link
Member

ferjm commented Apr 29, 2019

Thanks @PurpleHairEngineer

To squash the commits:

  • git rebase -i HEAD~3
  • On the rebase interactive tool you'll see the 3 commits and a commented (#-prefixed lines) description of what you can do.
  • In this case you want to add an s to the last 2 commits. Safe and exit.
  • In the next screen you can change the commit message. Write something descriptive enough like 'Add OscillatorNode::SetOscillatorType message'. Safe and exit.
@PurpleHairEngineer
Copy link
Contributor Author

PurpleHairEngineer commented Apr 29, 2019

@ferjm I followed your instructions to squash the commits and it gave me this message:

[detached HEAD d403149] Add OscillatorNode::SetOscillatorType message
Date: Sat Apr 27 10:41:05 2019 -0400
2 files changed, 18 insertions(+), 3 deletions(-)
Successfully rebased and updated refs/heads/OscillatorNodeMessage.

But I am not sure how to update this pull request to reflect the squashed commits.

I tried to do "git push -u origin OscillatorNodeMessage" as usual but it was rejected and said that the tip of my current branch is behind its remote counterpart.

@jdm
Copy link
Member

jdm commented Apr 29, 2019

@PurpleHairEngineer After modifying the branch history by squashing it, you need to force push (-f) to make the remote accept the new history.

@PurpleHairEngineer
Copy link
Contributor Author

PurpleHairEngineer commented Apr 29, 2019

@jdm Thank you! That worked.

@ferjm Okay, I have successfully squashed the commits and updated this pull request. It still says 2 commits, but the first one is because I had to update our fork to reflect the change to a flat directory structure.

@ferjm
Copy link
Member

ferjm commented Apr 30, 2019

That's ok. Thanks @PurpleHairEngineer :)

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Apr 30, 2019

📌 Commit d403149 has been approved by ferjm

@bors-servo
Copy link
Contributor

bors-servo commented Apr 30, 2019

Testing commit d403149 with merge 6e395ca...

bors-servo added a commit that referenced this pull request Apr 30, 2019
Oscillator node message

Implemented an audio node message that is specific to OscillatorNode (using BiquadFilterNode as a model) which updates the node's oscillator type.
@bors-servo
Copy link
Contributor

bors-servo commented Apr 30, 2019

☀️ Test successful - checks-travis
Approved by: ferjm
Pushing 6e395ca to master...

@bors-servo bors-servo merged commit d403149 into servo:master Apr 30, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
bors-servo added a commit to servo/servo that referenced this pull request May 21, 2019
Implemented type attribute for OscillatorNode interface

<!-- Please describe your changes on the following line: -->

Please see servo/media#242 for implementation of the new oscillator node message.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23282)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this pull request May 22, 2019
Implemented type attribute for OscillatorNode interface

<!-- Please describe your changes on the following line: -->

Please see servo/media#242 for implementation of the new oscillator node message.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23282)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.