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

rename Serre-Green-Naghdi source file #136

Merged
merged 2 commits into from
Aug 16, 2024
Merged

rename Serre-Green-Naghdi source file #136

merged 2 commits into from
Aug 16, 2024

Conversation

ranocha
Copy link
Collaborator

@ranocha ranocha commented Aug 16, 2024

No description provided.

@JoshuaLampert
Copy link
Owner

Thanks! Can you update the file name in equations/equations.jl please such that the tests don't fail?

Do you know if GitHub is smart enough to put my review comments in the SGN source file including suggestions in #135 in the correct places after merging this PR into main and main into hr/serre_green_naghdi or will they be outdated and thus not as easy to accept?

@ranocha
Copy link
Collaborator Author

ranocha commented Aug 16, 2024

Thanks! Can you update the file name in equations/equations.jl please such that the tests don't fail?

Sorry, should be done now...

Do you know if GitHub is smart enough to put my review comments in the SGN source file including suggestions in #135 in the correct places after merging this PR into main and main into hr/serre_green_naghdi or will they be outdated and thus not as easy to accept?

I don't know. Let's just try?

@JoshuaLampert
Copy link
Owner

I don't know. Let's just try?

I see you already pushed most of my suggestions in the other PR already. So we can't loose those. I just made some more suggestions that were still pending. Maybe we go through those first quickly before merging this PR such that they don't get lost if GitHub is not smart enough?

Copy link

codecov bot commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.00%. Comparing base (c5431b2) to head (8c3b29e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #136   +/-   ##
=======================================
  Coverage   97.00%   97.00%           
=======================================
  Files          18       18           
  Lines        1369     1369           
=======================================
  Hits         1328     1328           
  Misses         41       41           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JoshuaLampert
Copy link
Owner

CI failure looks spurious. So I'm going to merge this.

@JoshuaLampert JoshuaLampert merged commit 716f27c into main Aug 16, 2024
9 of 10 checks passed
@JoshuaLampert JoshuaLampert deleted the hr/rename_sgn branch August 16, 2024 17:36
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.

2 participants