-
Notifications
You must be signed in to change notification settings - Fork 365
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
Final test C10 #1431
Final test C10 #1431
Conversation
Co-authored by: Caden Carpenter <cadencarpenter63@gmail.com> Co-authored by: Anthony Theng <theng.anthonyy@gmail.com>
Hello, it appears you marked that the database is changed with this commit as well as the documentation needs an update with this merge. Can you confirm if this is the case? Furthermore, can you add your group mates using their github usernames via @[username]? I don't believe anyone has signed the CLA, please have everyone in the group do that. For @huss, please edit the PR to change Fixes #[962] to Partially Addresses #962. |
Hi, I don't believe our commit will change the database or require updated documentation. I'll edit the PR to add their usernames and make sure my group signs the CLA. |
All of the students have signed the CLA and everything in the code looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks to @heleeon, @CadenC63 & @anthonytheng for their first contribution to OED. I have a few comment to address. Please let me know if anything is not clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing all the comments. Testing and review found it works as desired. Congratulations to @heleeon, @CadenC63 & @anthonytheng on your first accepted work for OED.
Co-authored by: Caden Carpenter cadencarpenter63@gmail.com @CadenC63
Co-authored by: Anthony Theng theng.anthonyy@gmail.com @anthonytheng
Description
This pull request aims to allow testcase C10 to run alongside the other quantity meter tests.
It ensures that the Electric_Utility unit (BTU) is correctly correctly compared when read. The reading requested is in BTU.
This was accomplished through the specifications outlined in the DesignDocs
Partly addresses #962
(In general, OED likes to have at least one issue associated with each pull request. Replace [issue] with the OED GitHub issue number. In the preview you will see an issue description if you hover over that number. You can create one yourself before doing this pull request. This is where details are normally given on what is being addressed. Note you should not use the word "Fixes" if it does not completely address the issue since the issue would automatically be closed on merging the pull request. In that case use "Partly Addresses #[issue].)
Type of change
(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])
Checklist
(Note what you have done by placing an "x" instead of the space in the [ ] so it becomes [x]. It is hoped you do all of them.)
Limitations
No limitations we were made aware of.