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

1786 Adding real-to-real conversion builtin #2420

Merged
merged 28 commits into from
Jan 22, 2024

Conversation

oakleybrunt
Copy link
Collaborator

@oakleybrunt oakleybrunt commented Nov 27, 2023

Closes part of #1786

Copy link

codecov bot commented Nov 28, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e4e3176) 99.85% compared to head (c511ffb) 99.85%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2420   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         352      352           
  Lines       47299    47314   +15     
=======================================
+ Hits        47230    47245   +15     
  Misses         69       69           

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

@oakleybrunt oakleybrunt changed the title 1786 renaming conversion builtins - LFRicIntXKern, LFRicRealXKern 1786 Adding real-to-real conversion builtin Nov 28, 2023
@oakleybrunt oakleybrunt requested a review from TeranIvy December 1, 2023 14:30
@oakleybrunt oakleybrunt self-assigned this Dec 1, 2023
@oakleybrunt oakleybrunt added the LFRic Issue relates to the LFRic domain label Dec 1, 2023
Copy link
Collaborator

@TeranIvy TeranIvy left a comment

Choose a reason for hiding this comment

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

Nice work! I have some requests on testing and documentation, and there are some style issues to fix (see inline).

Tests with compilation pass, the examples build and the user guide builds and displays nicely. The changes are covered. The branch also needs to be updated with the latest master.

doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/developer_guide/APIs.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@TeranIvy TeranIvy left a comment

Choose a reason for hiding this comment

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

Almost there. There are some minor style and documentation changes to do.

Please also update the branch with the latest master if it changes before the next review round.

doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Show resolved Hide resolved
doc/user_guide/dynamo0p3.rst Outdated Show resolved Hide resolved
@TeranIvy TeranIvy added enhancement reviewed with actions Release Planning and creating PSyclone releases and removed under review labels Jan 15, 2024
Copy link
Collaborator

@TeranIvy TeranIvy left a comment

Choose a reason for hiding this comment

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

All well now. Updated changelog and user guide for merge to master. If CI and Integration tests pass, this can go on.

Copy link
Collaborator

@TeranIvy TeranIvy left a comment

Choose a reason for hiding this comment

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

All well with the integration tests, proceeding to merge.

@TeranIvy TeranIvy merged commit 893cfa0 into master Jan 22, 2024
12 checks passed
@TeranIvy TeranIvy deleted the 1786_renaming_conversion_builtins branch January 22, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement LFRic Issue relates to the LFRic domain ready to be merged Release Planning and creating PSyclone releases under review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants