Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

[common.php]: Ingest more years of data + tidy #4

Closed
wants to merge 1 commit into from

Conversation

zakgreant
Copy link

For the arrest and history chart functions, extend and condense the code to automatically ingest additional years of data, if they are available.

Also, a minor fix ($type was getting assigned a value within a conditional on line 74) and some code polishing.

What does this Pull Request do? ( required )

  • Extends common.php so that it can automatically ingest additional years of data for display.
  • Fixes a minor issue where $type gets assigned a value when it's clear that it's meant to be tested instead
  • Some code polishing - which you should totally discard if it isn't what you like ;)
    • For conditionals, moved the literals over to the left-hand side (eg. 'city' == $type vs $type == 'city') Helps ensure that variables aren't accidentally assigned when you don't mean to
    • Condensed some inline code into loops
    • Condensed blocks of if ... elseif .. into stacks of one-liners

Where should the reviewer start? ( required )

It's just one file, so it should be straight-forward to review.

How should this be manually tested? ( required )

I'd suggest diffing the outputs after a manual review, so as to ensure that I didn't accidentally bork something.

Your Contribution Contains: ( required )

  • New Components
  • New Documentation
  • New Unit Tests
  • Change to Existing Components
  • Change to Existing Documentation
  • Change to Existing Unit Tests

You have tested each of the following, and they work as expected: ( required )

  • npm test

Any background context you want to provide? ( optional )

I find the project and your work pretty amazing (and horrifying). I hope that you can get more data and that these little patches make it easier to make updates.

I'm happy to do more review and/or coding, if it's helpful.

What are the relevant GitHub issues? ( optional )

Ooof. Didn't check, actually.

Screenshots ( optional )

What GIF best describes this PR or how it makes you feel? ( optional )

For the arrest and history chart functions, extend and condense the code to be able to automatically ingest additional years of data, if they are available.

Also, a minor fix ($type was getting assigned a value within a conditional on line 74) and some code polishing.
@manifestinteractive
Copy link
Contributor

Just a heads up that we are working on a nationwide version of this project, so this code will be abandoned. It was a rushed hand coded proof of concept for just the state of CA. Now that we are going nationwide, we are switching over to a Laravel based framework that pulls data from an API. Once the official launch happens, this repo will be deleted and new documentation will be provided that points to the new nationwide repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants