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

Type stabilize offset_data #3273

Merged
merged 7 commits into from
Sep 28, 2023
Merged

Type stabilize offset_data #3273

merged 7 commits into from
Sep 28, 2023

Conversation

wsmoses
Copy link
Collaborator

@wsmoses wsmoses commented Sep 19, 2023

@navidcy
Copy link
Collaborator

navidcy commented Sep 19, 2023

@wsmoses, can you write a small description in the first comment summarizing what this PR do or fixes?

@glwagner
Copy link
Member

I think these minor changes allow / encourage the compiler to infer the output type of offset_data --- is that right? As far as I can tell it does not change the type that's outputted by offset_data. But I could be wrong.

Once we confirm the intent we can merge (it could make sense also to add a comment to the code so that this doesn't get reverted for some reason in the future). @wsmoses let us know if you'd like to tag a patch release as well.

@wsmoses
Copy link
Collaborator Author

wsmoses commented Sep 19, 2023

@glwagner yes indeed, the intent is to allow the compiler to infer the return (and internals) of offset_data. It does not change the result or result types.

@glwagner
Copy link
Member

Nice, let's merge ! Patch release or no? There will probably be a patch release before long anyways.

@navidcy
Copy link
Collaborator

navidcy commented Sep 19, 2023

We'll have a minor release soon with #3125, right? So no need to bother here with patch?

@navidcy
Copy link
Collaborator

navidcy commented Sep 28, 2023

@glwagner your comments implies you approve but you didn't formally approve. Is there something left to be done here?

@navidcy navidcy added the numerics 🧮 So things don't blow up and boil the lobsters alive label Sep 28, 2023
@glwagner glwagner merged commit 2a7d2a1 into CliMA:main Sep 28, 2023
47 checks passed
@glwagner
Copy link
Member

@glwagner your comments implies you approve but you didn't formally approve. Is there something left to be done here?

Nothing left. Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
numerics 🧮 So things don't blow up and boil the lobsters alive
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants