thegreenwebfoundation / co2.js

An npm module for accessing the green web API, and estimating the carbon emissions from using digital services
Other
389 stars 49 forks source link

v0.11.0 - Use Sustainable web design model by default #100

Closed fershad closed 2 years ago

fershad commented 2 years ago

This PR switches the default model used by CO2.js from OneByte to the newer Sustainable Web Design (SWD) model.

Users can now use the use the SWD model in two ways:

  1. By default const emissions = new co2()
  2. Explicitly const emissions = new co2({ model: "swd" })

Uses who still wish to use the OneByte model may do so by passing it as a parameter when initiating a new CO2 object.

const emissions = new co2({ model: "1byte" })

mrchrisadams commented 2 years ago

Thanks for this @fershad.

My only feedback on this is that we're matching for a string, 1byte, that might be spelled a number of different ways.

I think it's worth adding some logic for handling different variations of spelling like one-byte, onebyte, 1-byte, but having a try/catch statement, or at least some logging when someone doesn't match the two terms exactly.

This would at least offer some useful feedback when someone added a typo.

If you think try/catch is inappropriate, I think logging something with console.log or console.warn might help at least.

WDYT? 👍 ?

fershad commented 2 years ago

@mrchrisadams I'm leaning more towards throwing and error and providing the developer with as helpful of an error message as possible. This can include links to the appropriate docs pages where they can get more details. I feel this keeps things nice and tight, removing the need for us to think of similar edge cases when adding new models later.

As such 2c8ac17 includes changes that will:

  1. Throw an error if the model parameter passed in is not 1byte or swd (case sensitive).
  2. Throw an error if the developer is trying to use the perVisit() method, but that method is not available in the model being used. (i.e. using the 1byte model, and trying to use the perVisit method.

I've updated the tests with a new section that checks specifically for these error messages 56ebd09.

mrchrisadams commented 2 years ago

Hi Fershad, thanks for adding this - I'm much happier with the handling of errors here now :)

This looks good to me, but before we merge this into main, would you please look over how we can make a tag for v0.10 ?

you can see examples of tags below https://github.com/thegreenwebfoundation/co2.js/tags

That would make it easier to backport any changes we need to make later.

fershad commented 2 years ago

@mrchrisadams could you fill me in on what I'll be creating a tag for? Is it a tag like v0.11.0 with some notes attached? Or does that get created by the release step & you're asking for something different?

mrchrisadams commented 2 years ago

Hi @fershad - sorry about not being clear. I hadn't realised we already had tags set up for the v0.10.x releases already, and I wanted to capture the state of the latest v0.10 releases in case we had to make any changes later.

However, it looks like we already have these tags. Here's the output from git tag -l to list the tags in the repo:

# git tag -l

v0.0.2
v0.0.2-0
v0.1.0
v0.10.1
v0.10.2
v0.10.3
v0.10.4
v0.2.0
v0.4.0
v0.4.4
v0.4.5
v0.4.6
v0.4.7
v0.5.0
v0.6.0
v0.6.1
v0.7.0
v0.8.0
v0.9.0
v0.9.0-0

I was looking for the v0.10.x tags at the bottom, and had totally missed that they're all bunched up at the top between v0.1.0, and v0.2.0.

I also didn't quite understand how git tags are made as part of the np publishing process.

I think that as part of the process for making a release, the np module we use creates an annotated git tag anyway for the release we're publishing, so there's no extra work here for us to do - if we need to backport anything, we could check out the v0.10.4 tag and selectively apply commits if need be, then run a release to put out a v0.10.5 release for example if we wanted to.

It also turns out that we can make tags retroactively from any commit (see the docs on git tagging) - so merging this in PR into main, then making a release with our new np release for v0.11.0 ought to be enough to get the new version.

Basically, you're free to merge this in now - we just need to write a blog post explaining the updates to CO2.js, so there's a nice summary available, and then make a decision separately about when to make the next release to npm 👍 .

I'm happy to chat in Zulip/whereby tomorrow morning for a short call if you'd like too.