sopel-irc / sopel-weather

A working re-implementation of the weather plugin for Sopel
MIT License
6 stars 7 forks source link

show km/h and mph instead of m/s; add 'fc' alias for 'forecast' #34

Closed ghost closed 3 years ago

ghost commented 3 years ago

Description of the wind seems futile since people know what km/h and mph are. (Who the heck ever used m/s?) :)

Perhaps the formatting/output could be adjusted, idk.

Edit: This would close https://github.com/sopel-irc/sopel-weather/issues/15, imo.

ghost commented 3 years ago

Is the only complaint in that regard the removal of the description? I don't think anyone on the planet uses m/s for wind speed and see no value in that being an option.

More than happy to add back description if it gets the PR merged. 😛

RustyBower commented 3 years ago

The main one yes.

I'm trying to decide if having it display in imperial/better metric/both should be controllable via a configuration option as well (defaulting to both)

ghost commented 3 years ago

In general that seems like a good idea, but seems outside of the scope of this PR which just brings it up to speed on showing both imperial and metric, just like the temperature.

ghost commented 3 years ago

Also: Perhaps "readability" could be assisted by adding a : after description. e.g.:

Go from:

<Sackbot> Austin, Texas, US: 19°C (65°F), Clouds, Humidity: 80%, Light air 3.2km/h (2.0mph) (↓)

To:

<Sackbot> Austin, Texas, US: 19°C (65°F), Clouds, Humidity: 80%, Light air: 3.2km/h (2.0mph) (↓)

Thoughts?


Edit: Admittedly...no wind looks a little silly:

Los Angeles, California, US: 15°C (59°F), Clear, Humidity: 65%, Calm 0.0km/h (0.0mph) (↓)

But I think adding checks for 0 wind and adjusting the text and arrow accordingly also fall outside of the scope of this PR. :P


Edit 2: I suppose mph could just be an int. I don't know anyone who discusses wind speed in the US with decimals. 😅

RustyBower commented 3 years ago

Also: Perhaps "readability" could be assisted by adding a : after description. e.g.:

I'm happy with that

Edit: Admittedly...no wind looks a little silly:

Eh. 0.0 wind happens so infrequently...

Edit 2: I suppose mph could just be an int. I don't know anyone who discusses wind speed in the US with decimals. 😅

I'd rather keep it a float still, we can update it later if it looks silly

ghost commented 3 years ago

I'd rather keep it a float still, we can update it later if it looks silly

For perspective, this is the look with a couple of minor tweaks:

<Sackbot> Austin, Texas, US: 19°C (66°F), Clouds, Humidity: 77%, Light air: 5km/h (3mph) (↓)
<Sackbot> Los Angeles, California, US: 15°C (59°F), Clear, Humidity: 65%, Calm: 0km/h (0mph) (↓)

Edit: I think it looks much cleaner, but up to y'all (maintainers).

RustyBower commented 3 years ago

Looks fine to me. Go ahead and push up the latest and I'll merge it and cut a release

ghost commented 3 years ago

Should be good now. Definitely triple check, though; I am me, after all. :D