Closed koenjdejong closed 2 years ago
Als je millis al hebt, heb je secs niet meer nodig toch?
Ik geef later nog even grondig review, maar voor nu wil ik in ieder geval vast zeggen dat het er erg netjes uitziet!
De test files kan je weg doen als je toch geen tests gaat maken (hoeft ook niet)
Over dat milis gedoe, die heb ik ooit toegevoegd zodat het makkelijker zou zijn om de seconden en milis apart in te voeren op de website, en daarom komt hij terug in zowel de activerecord als db entry, omdat dat voor mijn gevoel de cleanste code zou opleveren
Apart invoeren kan prima met één veld in de database, dus ik zou het echt één column maken. Dan kan je daarna qua invoeren maken wat het fijnste werkt. Weet niet of twee velden makkelijker is, maar zo wel dan is het wat mij betreft geen reden om de DB er complexer van te maken. Ook weergave kan prima als het slechts als één waarde wordt opgeslagen.
Ik heb hem nu gefixt met een float, omdat dat eigenlijk nog logischer is naar mijn mening. 😆 Alle andere changes die je maakte zijn als het goed is ook verwerkt. Check ook maar ff wat ik nu met die stylesheet heb gefixt
Ik vind zelf de int logischer omdat je milis aan het bijhouden bent en dat worden nu opeens seconden maar wil dit ook goedkeuren. De stylesheet mag naar app/frontend/stylesheets
. Vroeger was dat de app/assets/stylesheets
maar dat is nu net anders omdat deze app webpacker gebruikt ipv de asset pipeline. Je hebt daar in frontend/packs
een entry point in application.css
Nu is alles correct als het goed is
Er uiteindelijk voor gekozen om niet voor alleen milis te gaan, omdat het nu decent werkt. Ook ff goed de migration checken, voor mij werkt ie maar weet niet hoe dat precies gaat als je hem gewoon zo merged. Issue #443