merge esgvoc in main #286 #287
Conversation
…dated exclusively for CMIP7
…oval_esgvoc Issue #276: Remove scenario experiments from esgvoc branch
matthew-mizielinski
left a comment
matthew-mizielinski
left a comment
There was a problem hiding this comment.
As far as I can tell this looks good, I think we need some more work on the README and some of the subfolders, but these should be broken out separately.
Could I request a squash and merge here as I don't think adding a complex git history here is worth the confusion.
wolfiex
left a comment
wolfiex
left a comment
There was a problem hiding this comment.
The content inserted needs to be reviewed and the scope discussed.
Contexts need to be correctly defined to link to the correct files.
It would be nice to have had the previous information present before the merge as to accurately access what information is being lost or replaced within each section. It is currently impossible to make an informed review without such a diff.
There was a problem hiding this comment.
Hard coded disabling is bad practice. You can disable the actions directly, specify when to run (ie not on main). Or define a global variable / file to determine what the action does.
There was a problem hiding this comment.
These are Variable Registry files and not CMIP7CVs specific.
Also the file content is missing for all of them.
There was a problem hiding this comment.
you have defined a prefix or key which does not appear in the files
There was a problem hiding this comment.
Model compnent links not specified in context. Current names are meaningless if not resolved.
There was a problem hiding this comment.
@ltroussellier looks mostly fine to me. Some thoughts:
- why are we keeping
.github? Let's just delete it and bring it back as either a) we have clear jobs or b) when Dan has time to bring thejson-ldbranch back in. As Dan says, hard-coding the disabling isn't ideal and, given we're not using these files, let's just delete them to avoid confusion (we can always get them back from git history)- as above for
_docs - this will solve Dan's points about the GitHub actions
- as above for
.python-versionprobably shouldn't be tracked? Remove and add to gitignore?- if it were fully up to me, I would delete all the content too i.e. only leave
README.mdand.gitignoreafter this merge. I think this is the truest reflection of what we are actually confident of i.e. nothing because everything is under review. It will be very simple to bring things back in, but I would start from zero so we know we have a clean base and can clearly track the changes as we add things back in- this will solve Dan's points about the content
|
Out of date and can be closed? |
Just need some rename and maybe some .github action deactivation