JuanIrache / DJI_SRT_Parser

Parses and interprets some data from DJI's Drones SRT metadata files.
https://tailorandwayne.com/dji-srt-viewer/
MIT License
47 stars 4 forks source link

New Features, some fixes and testing, testing, testing... #45

Closed GastonZalba closed 4 years ago

GastonZalba commented 4 years ago

Bugs

New features

Testings

JuanIrache commented 4 years ago

Hey, this is fantastic. Thanks!

Pitty that my initial code organization is not great (it was my first package), so updating it, reviewing changes, etc., is not super straightforward.

JuanIrache commented 4 years ago

Two issues I realised later. CircleCI integration breaks if the test command watches. I split the tests in two like so:

  "scripts": {
    "test-watch": "jest --watch",
    "test": "jest"
  },

Testing modifies the processed files, causing a ton of changes in git. Do you know why this is happening?

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   samples/processed_files/mavic_pro_CustomProperties.csv
        modified:   samples/processed_files/multi_mavic_pro_p4_rtk.csv
        modified:   samples/processed_files/multi_mavic_pro_p4_rtk.json
        modified:   samples/processed_files/multi_mavic_pro_p4_rtk.mgjson
        modified:   samples/processed_files/multi_mavic_pro_p4_rtk_MillisecondsPerSample(7000).json
        modified:   samples/processed_files/multi_mavic_pro_p4_rtk_Smoothing(0).json
        modified:   samples/processed_files/p4_rtk2.csv
        modified:   samples/processed_files/p4_rtk2.json
        modified:   samples/processed_files/p4_rtk2.mgjson
GastonZalba commented 4 years ago

I found this. Probably the new tests are adding LF (Unix) newlines to the files, and the existing ones are CRLF (windows). Maybe it's better add that folder to .gitignore?

JuanIrache commented 4 years ago

Yes.

Also, those files are not necessary for the automated test to succeed, are they? I'm assuming they are reviewed manually. Maybe we could have a separate script that creates them for manual review?

GastonZalba commented 4 years ago

Yes, they are for manual review. They have a condition that evaluate is there are errors upon writing but it can be skipped (the "real" tests are before that). Tomorrow i can fix this with the extra script.