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

Bug/236: Include all gridIntensity values in adjustments when only some are overridden #237

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

APJohns
Copy link

@APJohns APJohns commented Nov 7, 2024

Triage

Type of change

Please select any of the below items that are appropriate.

This pull request:

  • Adds a new carbon estimation model to CO2.js
  • Adds new functionality to an existing model
  • Fixes a bug with an existing model implementation
  • Add other new functionality to CO2.js
  • Add new data to CO2.js
  • Improves developer experience for contributors
  • Adds contributors to CO2.js
  • Does something not specified above

Related issue/s

Please list below any issues this pull request is related to.

Docs changes required

Do any changes made in this pull request require parts of the CO2.js documentation to be updated?

  • Yes
  • No
  • I don't know

If you answered "Yes", please create an corresponding issue in our Developer Documentation repository.

Describe the changes made in this pull request

When only defining some segment intensities on perVisitTrace, the variables.gridIntensity only has the explicitly set segments. This PR adds the default values.

I also moved the logic into a function to reduce repeated code and align all the segments.

Let me know if I've missed anything or if there is a better approach. Love this project, and I'm excited to contribute where I can!

`"${dataCenter.country}" is not a valid country. Please use a valid 3 digit ISO 3166 country code. \nSee https://developers.thegreenwebfoundation.org/co2js/data/ for more information. \nFalling back to global average grid intensity.`
);
adjustments.gridIntensity["dataCenter"] = {
value: SWDM3_GLOBAL_GRID_INTENSITY,
Copy link
Author

Choose a reason for hiding this comment

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

Was this meant to remain as SWDM3_GLOBAL_GRID_INTENSITY? By moving this into the function it will use globalGridIntensity like the others.

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.

1 participant