Skip to content

Prune generated tests #525

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

Closed
wants to merge 8 commits into from

Conversation

tmilnthorp
Copy link
Collaborator

Merge #524 first

Removing generated tests to match the reference files we have for generated code.

Adding BaseUnits class. Adding SI unit system.
Implementing IEquatable<BaseUnits> on BaseUnits, and adding some tests for BaseUnits. Adding some doc too.
…th quantity for example has a BaseUnits where only Length would be set. Only a UnitSystem needs it all. Moving logic there. Expanding UnitSystem tests and logic. Fixing WRC.
Fix for equality operator when both left and right are null
Better add test for != too, in case someone changes the != implementation
@tmilnthorp tmilnthorp changed the base branch from master to v4 October 16, 2018 19:04
@tmilnthorp tmilnthorp closed this Oct 16, 2018
@tmilnthorp
Copy link
Collaborator Author

That diff just looks ugly until #524 is pushed. Ill wait.

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

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

Nice. Changes look good, just waiting for merge. I wish github had some tooling to aid in these chained PR scenarios, something like target v4 branch, but only view changes on top of this other PR branch until it is merged

@angularsen
Copy link
Owner

Merged #526 instead.

@angularsen
Copy link
Owner

Btw, instead of creating a new PR you can use interactive rebase in git to remove unrelated commits and force push that to update the same PR.

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.

2 participants