search

microsoft / calculator

Windows Calculator: A simple yet powerful calculator that ships with Windows
MIT License
29.75k stars 5.39k forks source link

Add Angstroms Length-Converter option #2229

Closed DevBoiAgru closed 1 month ago

DevBoiAgru commented 2 months ago

Fixes #2175

Description of the changes:

How changes were validated:

DevBoiAgru commented 2 months ago

@microsoft-github-policy-service agree

DevBoiAgru commented 1 month ago

Hi @tian-lt I hope you're doing well, just wanted to ask if this small MR could be merged? It's a small and straightforward update.

Thanks for your time! 😊

tian-lt commented 1 month ago

Hi @DevBoiAgru, thank you for proposing this PR. Most of the changes look good to me. However, there's a test case failure found by the pipeline. It seems the MultipleConverterModeCalculationTest case may need some adjusts for the new unit. Could you take a look? Let me know when you fixed it, and then I'll merge the PR if everything goes well.

DevBoiAgru commented 1 month ago

@tian-lt Thanks for the reply. How could I run the tests locally on my machine so that I can mention you when it's done? I think the test case should be an easy fix with the order, so I dont want to push small changes again and again until it works

Update: Nevermind i think i figured it out, i'll keep you updated

tian-lt commented 1 month ago

Sure. You can run those unit tests by building the unit test project locally, which is located here: https://github.com/microsoft/calculator/tree/main/src/CalculatorUnitTests And, you may manually add a self-signed certificate to pack the unit test project to install it on your machine and then run for testing. The way to do that is exactly the same as what packing a side-load UWP app needs. Feel free to let me know if you need helps. @DevBoiAgru

DevBoiAgru commented 1 month ago

@tian-lt Apologies for another mention but the most recent commit should fix the test case not passing. I have modified a test case so that it checks for the correct units. Thank you.

tian-lt commented 1 month ago

LGTM. @hanzhang54 do you have any comments on this?