Khadija Saho work for CG11#1623
Conversation
|
@Khadijasaho First, I want to apologize for the delay in responding to this PR due to recent issues. Thank you for this contribution. OED's records do not indicate that you have signed the CLA but the box is checked in the PR description. Could you either do it or let me know that you think OED's records are incorrect. Also, OED needs you to add to the description or via a comment that either you did all the work by yourself or give the names/GitHub IDs for anyone else involved. Once this is done, the PR can be reviewed. Please let me know if anything is not clear. |
|
@huss, I have signed the OED CLA, I think it probably did;t go through. I also added my summary of what the test I added does and what I have added. |
@Khadijasaho Thank you for the updates and I see your CLA. Your description uses "I" but I just want to verify whether this was only your work or if someone else was involved since often they are done by teams. Thanks. |
|
Yes, on my own. I have a delay due to a conflict in school and a personal issue in completing my part. @huss |
|
This was somewhat out of date with the development branch so I merger that to get rid of the startup issue that was recently merged. No merge conflicts and I pushed the merge to the PR. |
huss
left a comment
There was a problem hiding this comment.
@Khadijasaho That you for this contribution. Review and testing found it works as desired. I made one small comment to consider but this is almost ready to merge. Please let me know if anything is not clear or you have thoughts.
| @@ -267,6 +267,73 @@ mocha.describe('readings API', () => { | |||
|
|
|||
| // Add CG11 here | |||
There was a problem hiding this comment.
Can you please remove this placeholder comment since you added the test.
Description
(Please include a summary of the change and which issue is touched on. Please also include relevant motivation and context.)
I completed the CG11 task assigned to me, and I worked on it on my own.
What this test does:
CG11 verifies that the compare readings API correctly returns energy data for a group when using a reverse unit conversion path (MJ → kWh) to display values in BTU. It checks a 1 day shift ending at 2022-10-31 17:00 using 15 minute reading intervals.
What I added:
Defined unit u3 (MJ) and unit u16 (BTU)
Defined conversion c6 (MJ → kWh, slope 1/3.6), the reverse direction of the standard kWh → MJ conversion
Defined conversion c3 (MJ → BTU, slope 947.8)
Loaded all units and conversions alongside the standard unitDatakWh and conversionDatakWh
Queried the compare readings API for a group with BTU as the display unit
Fixes #[issue]
(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
(Describe any issues that remain or work that should still be done.)