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

Extend Fraser/Peace/Columbia streamflow and fix the crash loop #468

Closed
wants to merge 12 commits into from

Conversation

Nospamas
Copy link
Contributor

@Nospamas Nospamas commented Dec 10, 2024


Secondary changes:

  • Does some package updates (audit fix, prettier)
  • Adds some VSCode launch capabilities for debugging

Available for testing on beehive

@Nospamas Nospamas changed the title WIP: Extend James's fraser streamflow to fix the crash loop Extend James's fraser streamflow to fix the crash loop Dec 10, 2024
@jameshiebert jameshiebert changed the title Extend James's fraser streamflow to fix the crash loop Extend Fraser/Peace/Columbia streamflow and fix the crash loop Dec 11, 2024
@jameshiebert
Copy link
Contributor

I read over your code changes and it all looks good. The state changes are the bulk of the work... they're a bit verbose, but if they align with the react-state recommendations, then I'm happy :)

I'm always suspicious of any PR that does multiple things, and this one does the thing that I initially set out to do (extend the streamflow frontend to another watershed) and the bitrot problem (the crash loop).

Would you be able to rebase your commits on to our main branch and confirm that the crash loop is fixed under those circumstances? Then add the watershed extension commits?

@@ -120,7 +129,7 @@ export default class WatershedSummaryTable extends React.Component {
}

// Waiting for data
if (this.state.fetchingData || this.state.data === null) {
if (!this.state.data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This appears to work great both at the outset and if a user clears a previously selected watershed outlet.

@Nospamas
Copy link
Contributor Author

I read over your code changes and it all looks good. The state changes are the bulk of the work... they're a bit verbose, but if they align with the react-state recommendations, then I'm happy :)

I'm always suspicious of any PR that does multiple things, and this one does the thing that I initially set out to do (extend the streamflow frontend to another watershed) and the bitrot problem (the crash loop).

Would you be able to rebase your commits on to our main branch and confirm that the crash loop is fixed under those circumstances? Then add the watershed extension commits?

I've cherry picked out those commits unrelated to the fraser river extension into #469

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.

2 participants