Skip to content

Conversation

@ramramps
Copy link
Collaborator

Purpose

Task : https://jira.autodesk.com/browse/QNTM-1304

This PR fixes the bug in conversion between CubicMillimeter to CubicMeter

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@mjkkirschner

converterNode.SelectedFromConversion = ConversionUnit.CubicMillimeter;
converterNode.SelectedToConversion = ConversionUnit.CubicMeters;
ViewModel.HomeSpace.Run();
Thread.Sleep(500);
Copy link
Member

Choose a reason for hiding this comment

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

did you find this was required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know whether this is required. May be I should remove this and run the self serve CI. This code is 3 year old now.

Copy link
Member

Choose a reason for hiding this comment

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

I'm confused, this looks like new code you added?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope..the other thread.sleep code. I removed this, and running self serve CI

Copy link
Member

Choose a reason for hiding this comment

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

oh I see, that was following the pattern.

@mjkkirschner
Copy link
Member

LGTM (if that test passes)

@ramramps
Copy link
Collaborator Author

Tests passed.

11940 tests
0 test errors
0 test failures
26 tests ignored

Merging this.

@ramramps ramramps merged commit c5bee0e into DynamoDS:master Mar 14, 2018
@ramramps
Copy link
Collaborator Author

oh man..thread.sleep is not there in this...reverting this

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