Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update server files to remove extra whitespaces #49

Merged
merged 16 commits into from
Dec 24, 2021

Conversation

Esteban82
Copy link
Member

See #38.

I update the AF files. Please let me know if I make the PR ok (it is the first time I am doing it from VSC).

@Esteban82
Copy link
Member Author

I use this command to remove the extra whitespaces:
sed -i "s/^ *//;s/ *$//;s/ \{1,\}/ /g" *.txt

And this to reduce the decimals to 6 values
gmt convert old.txt -fg --IO_COL_SEPARATOR=space --FORMAT_FLOAT_OUT=%.6lf > new.txt

The later was only applied to: BV, ER, ET, SD, SS, TF.

Copy link
Member

@PaulWessel PaulWessel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks fine.

@Esteban82
Copy link
Member Author

I see that there is a conflict. It seems my local branch don't have the latest changes.

For the RU.txt and CHU.txt files I manually updated. The same for the DE.txt and NL.txt (in Europe).

Not sure how should I proceed.

Copy link
Member

@PaulWessel PaulWessel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am basically approving these blind since it is too much work to build the netcdf and a new version and test it...

@Esteban82 Esteban82 changed the title WIP Update server files Update server files Dec 23, 2021
@Esteban82
Copy link
Member Author

I finished updating all the files. For the 5 files that have conflicts (RU, CHU, DE, NL, FJ) I carefully modify them considering the last changes.

@Esteban82
Copy link
Member Author

@PaulWessel maybe now you could create the netcdf to test it.

@PaulWessel
Copy link
Member

Will try tomorrow Thursday.

@Esteban82
Copy link
Member Author

Will try tomorrow Thursday.

Ok. This is the checksum of the tar.gz file: 42c6ee7529a3556b315646fb14cbf463.

And this the figure that I get:

dcw-figure

@PaulWessel
Copy link
Member

Looks like you either did not update master before you started on this branch? The config.mk file says 2.0.1, for instance while master says 2.0.2.
I think if you are sure those 5 files are correct then look at how to merge while forcing the new files to be taken as is. Maybe either @meghanrjones or @seisman can advice on git stuff - I am just a git abuser.

@maxrjones maxrjones changed the title Update server files Update server files to remove extra whitespaces Dec 23, 2021
@maxrjones
Copy link
Member

There's two options given the size of the changes:

  1. Merge the master branch into Reformat-orig-files using the versions of RU.txt, CHU.txt, DE.txt, NL.txt, and FJ.txt that are in Reformat-orig-files (this assumes that all changes from the master branch have been made independently here).
  2. Merge the master branch into Reformat-orig-files using the versions of RU.txt, CHU.txt, DE.txt, NL.txt, and FJ.txt that are in the master branch and then re-run the formatting steps on those five files.

Would you want me to take one of those actions to resolve the merge conflicts, @Esteban82?

@Esteban82
Copy link
Member Author

Let's do 2) to be completely sure.

@maxrjones
Copy link
Member

OK, I merged in master accepting all incoming changes. You'll need to do a git pull before reformatting the 5 files that had merge conflicts.

@Esteban82
Copy link
Member Author

I think that everything is ready now.

@Esteban82
Copy link
Member Author

Should we write something in the changelog?

@PaulWessel
Copy link
Member

You could add an entry like "Reformat original files using consistent format and white-space" so that in the future we at least will remember this happened should something odd show up later

@PaulWessel
Copy link
Member

So do that, and then you can merge since already approved.

@Esteban82 Esteban82 merged commit e63b1da into master Dec 24, 2021
@Esteban82 Esteban82 deleted the Reformat-orig-files branch December 24, 2021 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants