search

loganoz / horses3d

HORSES3D: A high-order discontinuous Galerkin solver for flow simulations and multi-physics applications
https://loganoz.github.io/horses3d/
MIT License
112 stars 24 forks source link

Stefano development #132

Closed stecolumbus closed 1 year ago

stecolumbus commented 1 year ago

@Andres-MG , @oscarmarino . I cloned the old version of the code before the Andre's changes. Can you please checked that everything is ok?

Andres-MG commented 1 year ago

I think all of my changes are there, but there are 346 files to review so I might have skipped something. Most of the changes that I see are simply trailing whitespaces though, which is already inherent to HORSES since nobody removes them before pushing something to master 😅.

Also, we could probably rebase all the "merge branch" commits into one to leave a cleaner history when we merge this into master. Ask me if you want to do it but don't know how.

stecolumbus commented 1 year ago

It should be fixed, it compiles with gfortran 9.3.0 and should be ok with all the next versions.

loganoz commented 1 year ago

@Andres-MG, well the idea is that we do not go back to what we had before https://github.com/loganoz/horses3d/commit/67933852ed78d61ef1af770e8d8102215dda7717. @stecolumbus implemented again his IBM stuff but retaining the new stuff from master. Please, double check that your new stuff is here and that all is working fine. If all is fine, this is the version for the journal.

loganoz commented 1 year ago

@Andres-MG , about the rebase I think it is a good idea. I think this would help a lot with the do/undo mix we have right now. Could you help with that? I have never done it myself. Finally, any idea on what happens at the end of the files? This is a pain in the ass, as many of the files were not really modified.

Andres-MG commented 1 year ago

I can help rebasing this branch, but what is in master should stay as it is. Otherwise, people will have a mess of files belonging to a non-existent branch.

loganoz commented 1 year ago

I've been thinking. Tomorrow I'll try to clone this version locally and substitute (copy/paste) all the files in a master version. With this we should have all the changes in a single commit, with clear view of the modifications. I'll also try to get rid of the last line of the file change.

Andres-MG commented 1 year ago

The rebase seems to require a lot of work, there are hundreds of commits and they all seem to conflict. IMO we should use git rebase instead of git merge to sync non-master branches to avoid this cluttering of merge commits, and then use merge only to go from our branches to master.

Putting all this in a single commit looks like a good option to me too. As for removing trailing whitespaces and lines, I don't think there is a quick way of doing it... We could start using a formatter (like this) to make sure that the code we push complies with some style rules, but that is not something we can do for this merge.

loganoz commented 1 year ago

I will close this pull request, as it is substituted with the one commit.