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

Refactoring of vector #415

Merged
merged 19 commits into from Mar 30, 2020
Merged

Refactoring of vector #415

merged 19 commits into from Mar 30, 2020

Conversation

@Mingun
Copy link
Contributor

Mingun commented Mar 29, 2020

Some refactoring of Vector2D and Vector3D. Summary of changes:

  • Add all missing documentation (deny(missing_docs) is not a problem for vectors now)
  • Implement missing *Assign operators
  • Implement Round/Floor/Ceil/Zero
  • Add some more tests
  • Reorder methods thus documentation looks more organic and mostly all methods have only necessary bounds
Copy link
Collaborator

nical left a comment

Lots of very good stuff in here, thanks!

Just a small change in the abs comments and this is good to go.

src/vector.rs Outdated Show resolved Hide resolved
src/vector.rs Outdated Show resolved Hide resolved
@Mingun
Copy link
Contributor Author

Mingun commented Mar 30, 2020

Done. I've replace incorrect notice with examples of current behavior

///
/// Method panics if `abs()` of underlying type panics:
///
/// ```rust,should_panic

This comment has been minimized.

@nical

nical Mar 30, 2020

Collaborator

This only panics debug builds, you are adding a test that implies that abs always panics with . Please just remove this section, it is too specific and describes something that is outside of euclid. If you really want to be explicit about the behavior of abs, you can write "The behavior for each component follows the scalar type's implementation of num_traits::Signed::abs."

This comment has been minimized.

@Mingun

Mingun Mar 30, 2020

Author Contributor

Ok, done.

@Mingun Mingun force-pushed the Mingun:vector branch from b6081b6 to df6539a Mar 30, 2020
@nical
Copy link
Collaborator

nical commented Mar 30, 2020

Thanks! @bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Mar 30, 2020

📌 Commit df6539a has been approved by nical

@bors-servo
Copy link
Contributor

bors-servo commented Mar 30, 2020

Testing commit df6539a with merge eb32d41...

@bors-servo
Copy link
Contributor

bors-servo commented Mar 30, 2020

☀️ Test successful - checks-travis
Approved by: nical
Pushing eb32d41 to master...

@bors-servo bors-servo merged commit eb32d41 into servo:master Mar 30, 2020
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
@Mingun Mingun deleted the Mingun:vector branch Mar 30, 2020
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

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