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

Update JSDocs to match behaviour #198

Merged

Conversation

mgriffin-scottlogic
Copy link
Contributor

Added new JSdoc typedef for components of CO2 estimate
Added it as a union for return types that can be total or components
Addresses part of #196

Added it as a union for return types that can be total or components
src/co2.js Outdated
@@ -83,7 +92,7 @@ class CO2 {
*
* @param {number} bytes
* @param {boolean} green
* @return {number} the amount of CO2 in grammes
* @return {number|CO2EstimateComponents} the amount of CO2 in grammes or its separate components
Copy link
Contributor

Choose a reason for hiding this comment

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

i think there are cases where perVisit has a different return type than perByte:

const co2Emission = new co2({
  results: "segment",
});

const bytesSent = 1000 * 1000 * 1000;
const greenHost = false;

console.log(co2Emission.perVisit(bytesSent, greenHost));

the result i'm seeing looks like this:

{
    "consumerDeviceCO2 - first": 154.825749,
    "consumerDeviceCO2 - subsequent": 1.0321716600000002,
    "networkCO2 - first": 41.68385550000001,
    "networkCO2 - subsequent": 0.27789237000000006,
    "productionCO2 - first": 56.570946750000004,
    "productionCO2 - subsequent": 0.3771396450000001,
    "dataCenterCO2 - first": 44.66127375,
    "dataCenterCO2 - subsequent": 0.297741825,
    "total": 299.7267705
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot, I'll have a look at that - might need a unique type for perVisit.
Is there possibly also a bug there? Adding up all of the first values only gives ~297.74, so I assume the total is including the subsequent visit information too. In terms of what you might want to use this for it would make more sense to have "total - first" and "total - subsequent" properties. Or use sub-objects with first and subsequent values for all of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NVM, I see that both parts of the component are reduced by a percentage so the total is correct. Having separated total components would be a feature request/matter of taste.

@fershad fershad merged commit a6f1d4d into thegreenwebfoundation:main Apr 4, 2024
3 checks passed
@mgriffin-scottlogic mgriffin-scottlogic deleted the component-docs branch April 8, 2024 10:21
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.

3 participants