search

frc-frecon / frecon

An API for building scouting apps for FRC competitions
MIT License
3 stars 0 forks source link

Scraper is Convoluted #93

Open rye opened 8 years ago

rye commented 8 years ago

As of a75d272bd14685b52a9c8e0f4ef5fbc172ddc4dd, scraper.rb is a bit odd. @Sammidysam can provide some insight, since he is the only person that has touched it thus far. We currently seem to get the warning about the "Empty Array" (which is emitted from around line 29)

In our discussion, one suggestion is to clean this up and make it slightly less odd. We might also add a File scraper to simplify importing from JSON dumps, since that is something we do a lot. (More-so than over the network, it seems)

Sammidysam commented 8 years ago

The only problem I had with it at all is that there is an elsif line with no clause after it. This to some extent is why all data is called empty. I just want to fix that bug up at some point, and then I'm cool with the functionality of the class.

-Sam

Sent using CloudMagic Email [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=8.2.23&pv=5.1&source=email_footer_2] On Tue, Mar 15, 2016 at 4:01 PM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: As of a75d272 [https://github.com/frc-frecon/frecon/commit/a75d272bd14685b52a9c8e0f4ef5fbc172ddc4dd] , scraper.rb [https://github.com/frc-frecon/frecon/blob/a75d272bd14685b52a9c8e0f4ef5fbc172ddc4dd/lib/frecon/scraper.rb] is a bit odd. @Sammidysam [https://github.com/Sammidysam] can provide some insight, since he is the only person that has touched it thus far. We currently seem to get the warning about the "Empty Array" (which is emitted from around line 29 [https://github.com/frc-frecon/frecon/blob/a75d272bd14685b52a9c8e0f4ef5fbc172ddc4dd/lib/frecon/scraper.rb#L26-L30] )

In our discussion, one suggestion is to clean this up and make it slightly less odd. We might also add a File scraper to simplify importing from JSON dumps, since that is something we do a lot. (More-so than over the network, it seems)

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/issues/93][https://github.com/notifications/beacon/AC7z1lFCp0k7L0_yVNCqBlT2pbr53sccks5ptxAdgaJpZM4HxZAv.gif]

rye commented 8 years ago

Okay. We can add logic—is that all that is needed?

Sammidysam commented 8 years ago

I believe so. Really the only problem I thought was that it would say the data was empty when it wasn't. Were there other problems?

-Sam

Sent using CloudMagic Email [https://cloudmagic.com/k/d/mailapp?ct=pa&cv=8.2.23&pv=5.1&source=email_footer_2] On Wed, Mar 16, 2016 at 11:09 AM, Kristofer Rye < notifications@github.com [notifications@github.com] > wrote: Okay. We can add logic---is that all that is needed?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub [https://github.com/frc-frecon/frecon/issues/93#issuecomment-197374723]