wmo-im / BUFR4

BUFR edition 4

MIT License

27 stars 9 forks source link

161 proposal for new bufr table entries for eps sg eumetsat iasi ng level 2 and vii level 2 #174

Closed amilan17 closed 12 months ago

amilan17 commented 1 year ago

161

antoinemerle commented 1 year ago

Thanks @amilan17.

amilan17 commented 1 year ago

@antoinemerle I made some editorial changes (see https://github.com/wmo-im/BUFR4/compare/de70ce8e56c2f2a5fee11be858b5a9860c398521...161-proposal-for-new-bufr-table-entries-for-eps-sg-eumetsat-iasi-ng-level-2-and-vii-level-2)) and have some comments for your consideration.

Thanks.

antoinemerle commented 1 year ago

Hi @amilan17 ,

Quick question : When you mean "Have comments for your consideration" you mean that probably I "click" on the Ready for review ? I am not able to see any of your comments apart the changes you've made

Second point which is interesting for my future self , I got an automated scripts that push the changes inside the PR branch. I would be interested that you confirm the two follow assumption :

table should always start with capital 'T' : "Table"
the following char "-" is prohibited in code name

thanks a million for confirming that, I will update my script accordingly :)

amilan17 commented 1 year ago

@antoinemerle

Quick question : When you mean "Have comments for your consideration" you mean that probably I "click" on the Ready for review ? I am not able to see any of your comments apart the changes you've made

I forgot to submit my review -- you should see the comments now.

table should always start with capital 'T' : "Table"

T should not be capitalized when referencing "Code table" and "Flag table"

the following char "-" is prohibited in code name

No, it's not prohibited. I removed it in "Water-vapour" for consistency with same term in other tables

antoinemerle commented 12 months ago

Hi @amilan17 ,

I implemented all the comment's you've made. Really glad that you made this review and gave me feedback. I am taking in account all the notes and advices you gave me.

Please do not hesitate to check again for consistency the full sequence.

On my side I will review and go through my proposal to implement the following across the sequences. replacing the

Set to "something" with 9

=9 "Something"

amilan17 commented 12 months ago

@antoinemerle thank you for your updates. I made some copyedit changes that I'd appreciate you taking a quick look at. 89073a1

antoinemerle commented 12 months ago

HI @amilan17 , Thanks a million time for your review. I am really sorry that you spent so much time to edit my PR....

I think we (the group) can try to :

have a tiny pdf guideline for anyone pushing data in the branch in order to know the correct "nomenclature" to follow to respect the WMO one.
have a simple pdf guide on how to use github and specific branch.

On my side, I am going to implement all the things we saw together in my SW that is converting my csv table in the WMO ones...

Thanks again

I review your changes, and it is fine according to me. Thanks a million for chaning the SO2 to sulphur dioxide in the title.