Closed jonssonchristian closed 4 months ago
Hi @christianjnaturalpower,
Just very quick comments, I haven't reviewed it fully.
The documentation files are all created automatically. Therefore no need to edit those. Could you remove those from this PR?
The create SQL statement file under Tools could also be updated to include this. See the contributing guidelines for the files to update: https://github.com/IEA-Task-43/digital_wra_data_standard/blob/master/contributing.md
And as I read it there myself, the CHANGELOG file too.
Cheers,
Thanks @stephenholleran.
The documentation files are all created automatically. Therefore no need to edit those. Could you remove those from this PR?
I suspected that might be the case. I have removed the updates to the docs file now.
The create SQL statement file under Tools could also be updated to include this. See the contributing guidelines for the files to update: https://github.com/IEA-Task-43/digital_wra_data_standard/blob/master/contributing.md
The file iea43_wra_data_model_postgresql.sql already includes creating a uuid
field in the measurement_location
table. I could not see anywhere else where it would need to be updated. Let me know if I have missed something. I did not see any code that reads data from a JSON document and inserts it into the reference SQL database. If there is something like that I presume that also needs to be updated.
And as I read it there myself, the CHANGELOG file too.
Updated.
Hey @jonssonchristian,
We have agreed to do a new release with the changes we have made over the last year. I would like to include this one so your IEC group will have it. Please remind me if this is still good to go and I just have to review it or was there something else you were to do?
Cheers,
Hi @stephenholleran,
That sounds great. The PR includes everything I think needs to be updated to add the location uuid
field. I pulled the latest version of the dev
branch now and merged it into the branch for this PR.
Hi @stephenholleran, that all looks good to me, thank you. From my perspective you can go ahead and merge it.
This PR provides the implementation for issue #232.
It includes the following additions:
I have checked that all updated JSON examples validate successfully against the updated JSON Schema.
I tried to cover all the places that should be updated, but may have missed something. I did not add anything under the 'app' as I have the impression that creates the form dynamically from the content of the JSON Schema (but maybe I am wrong there). I did not update anything in the SQL files since they already include a 'uuid' field in the 'measurement_location' table.
I apologise I had not read the contributing guidelines properly before I created the branch, and missed that I should have tagged individual commits and the branch with the issue number. I will do that next time.