Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
| <td align="center"><a href="http://openclimatefix.org"><img src="https://avatars.githubusercontent.com/u/38562875?v=4?s=100" width="100px;" alt=""/><br /><sub><b>dantravers</b></sub></a><br /><a href="#ideas-dantravers" title="Ideas, Planning, & Feedback">🤔</a></td> | ||
| <td align="center"><a href="https://github.com/peterdudfield"><img src="https://avatars.githubusercontent.com/u/34686298?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Peter Dudfield</b></sub></a><br /><a href="https://github.com/openclimatefix/nowcasting/commits?author=peterdudfield" title="Code">💻</a></td> | ||
| <td align="center"><a href="https://github.com/braddf"><img src="https://avatars.githubusercontent.com/u/41056982?v=4?s=100" width="100px;" alt=""/><br /><sub><b>braddf</b></sub></a><br /><a href="https://github.com/openclimatefix/nowcasting/commits?author=braddf" title="Code">💻</a></td> | ||
| <td align="center" valign="top" width="14.28%"><a href="http://tanner.me"><img src="https://avatars.githubusercontent.com/u/227?v=4?s=100" width="100px;" alt="Damien Tanner"/><br /><sub><b>Damien Tanner</b></sub></a><br /><a href="#projectManagement-dctanner" title="Project Management">📆</a></td> |
There was a problem hiding this comment.
These must have been merged back through from main, not my doing but seems fine
| <td align="center" valign="top" width="14.28%"><a href="https://github.com/rachel-labri-tipton"><img src="https://avatars.githubusercontent.com/u/86949265?v=4?s=100" width="100px;" alt="rachel tipton"/><br /><sub><b>rachel tipton</b></sub></a><br /><a href="https://github.com/openclimatefix/quartz-frontend/pulls?q=is%3Apr+reviewed-by%3Arachel-labri-tipton" title="Reviewed Pull Requests">👀</a> <a href="https://github.com/openclimatefix/quartz-frontend/commits?author=rachel-labri-tipton" title="Code">💻</a></td> | ||
| </tr> | ||
| </tbody> | ||
| </table> |
There was a problem hiding this comment.
you could always merge main into development, so these dont appear in the PR
There was a problem hiding this comment.
Done and Github weirdness sorted now 👍
| console.log("setting aggregation to gsp"); | ||
| setAggregation(AGGREGATION_LEVELS.GSP); | ||
| } else { | ||
| console.log("setting aggregation to site"); |
There was a problem hiding this comment.
im not totally sure, but do you need this console.logs, I understand they are a bit like print statements in python, good for developing, but perhaps not needed for production code?
There was a problem hiding this comment.
Yeah, we could be a little more critical of where these are; for more complex behaviours I sometimes keep them in because it's very hard to debug in live otherwise. With the next refactors I'll definitely try to remove some more noise / untidiness like this...
| - [ ] I have made corresponding changes to the documentation | ||
| - [ ] I have added tests that prove my fix is effective or that my feature works | ||
| - [ ] I have checked my code and corrected any misspellings | ||
| - [ ] I have updated the release docs with the contents of this release |
There was a problem hiding this comment.
nice, good to get this here, rather than copying it
peterdudfield
left a comment
peterdudfield
left a comment
There was a problem hiding this comment.
Looks really good, ive left one small comment.
I had look on here and tested the site aggrgation level, and it looks good. The lock icon looks good, make it clear. I was slightly confused by the lock on "Auto" but I think it doesnt make sense.
2 participants
Pull Request
Description
Selection of small tasks bundled in together:
Fixes #439
Fixes #649
How Has This Been Tested?
If your changes affect data processing, have you plotted any changes? i.e. have you done a quick sanity check?
Checklist: