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

Added ConvertMetresToLengthUnits() Method #4

Merged
merged 5 commits into from
Jan 15, 2022

Conversation

hartkemd
Copy link
Contributor

Hi, Pierre. I made a method that converts metres to other units of length. I'm sure it's not totally perfect, but it's something we can start with, and I think it's in the spirit of what you were doing. I also renamed the Length enum to LengthUnits, which seemed to make more sense to me. If you don't like it, you're welcome to change it back. One last thing I did was to re-wire the UI work I did previously, to allow us to test the new conversion method I added.

…th enum to LengthUnit, to be more clear. Re-wired some UI.
@hartkemd
Copy link
Contributor Author

Sorry, I think I should have split up my commits into more bite-sized chunks.. I'll try to do that next time.

@pjlplourde
Copy link
Owner

I don;t think this one fits with the direction I was going and has a number of conflicting files. If you want to refresh your fork and justg add your ConvertMetresToLengthUnits method as a separate method called from ConvertLength when the input units are metres, that would work as a good model to follow for the rest of the conversions.

@hartkemd
Copy link
Contributor Author

Sorry, took me a while to wrap my head around what was going on in Git, but I think I got it.
Is something like this what you had in mind?

@pjlplourde pjlplourde merged commit dcf1000 into pjlplourde:master Jan 15, 2022
@pjlplourde
Copy link
Owner

Thank you for the work. It's appreciated.

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.

None yet

2 participants