FirstLegoLeague / clock

Countdown clock for competition area
GNU General Public License v3.0
3 stars 15 forks source link

Referees 2018/configurable files #20

Closed idanstark42 closed 6 years ago

idanstark42 commented 7 years ago

This is what we talked about. All is configurable via the configuration file or the configuration view (i think I did this correctly)

rikkertkoppes commented 7 years ago

In general it look pretty much ok, I have made some remarks here and there.

Apply a linter and a jsformatter to the code. There are a few styling issues:

What is the significance of the +6 suffix in the start track file name?

Lastly, add a bit of readme on how the track config works, which events are available

Overall, pretty nice work. This brings a lot more flexibility to the clock

idanstark42 commented 7 years ago

Thank you for the review. I took your notes and made changes. The +6 on the file was there before... I just didn't remove it. All other things I hope I changed correctly

rikkertkoppes commented 7 years ago

Checked the pr, left a few comments. Basically, the branch does not work in this state. Lots of js errors, I have identified a few in the comments

rikkertkoppes commented 7 years ago

Ok, I got stuff kinda working now, but there is still a lot wrong with it. You might want to either start manual test scripts or automated tests for it all. The stuff added is rather complicated and a lot can go wrong with it.

The stuff I found thus far:

idanstark42 commented 7 years ago

I think I fixed everything... Tell me if not. About the third point (the pause and unpause). If you attached a track to the pause event, i.e. start on pause then it'll start when you pause, upause is a different event. But I think everything else is better now

rikkertkoppes commented 7 years ago

There is still a lot wrong here. These scenarios still don't work: