Codecov Report

Base: 64.00% // Head: 64.00% // No change to project coverage 👍

Coverage data is based on head (b03d318) compared to base (fb55b8f).
Patch has no changes to coverable lines.

@@           Coverage Diff            @@
##           develop     #216   +/-   ##
========================================
  Coverage    64.00%   64.00%           
========================================
  Files           18       18           
  Lines         1328     1328           
========================================
  Hits           850      850           
  Misses         478      478           

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

No test though...

Thanks, thanks.

Sorry i didn't catch it in time, but you should make aliases of the functions and deprecate them when you change the api like that.

Probably there's a similar issue with the C part we should check (more familiar with deprecating on C side tbh). Thankfully you can map it to a different function with bind(c, name=function_name_old) if necessary. Although when deprecating, we should do the other way around.

Thanks @LecrisUT for your comment. Fortran support of spglib is fairly weak. I'm not familiar with it but no maintainer. This PR is a fix of the fortran wrapper. This function in C was changed at v2 from v1, but the fortran wrapper was not updated following the change.

I'm not also good at those stuff like deprecating manner. Could you tell me a little bit more in detail? I also have to make the fortran wrapper of new functions made at v2. So your information is helpful.

I didn't know this was part of the major release change. In that case I think it would be fine.

Normally, especially between minor version changes, when you need to change an api function you would:

• Create a new function, preferably with a different name
  • Should avoid names like function_new because that will become confusing quick after more updates
  • If possible separate namespaces/header files by version, e.g. spglib/v1/spglib.h
• Mark old function as deprecated with [[deprecated]]
  • Unfortunately plain C only has this feature in C23, while C++ had it since C++14. So for the time, it should be sandwiched by #ifdef to check for C++ or C23. For the developers using older compilers, there is little we can do.
  • Internally call the new function with reasonable default values
  • Allow for 1-2 minor versions before removing in a major release change. You could also remove the old apis after a few minor version changes
• If possible, or if there is a major functionality change, keep the old function name with [[deprecated]] and mark the function as deleted (e.g. void foo(int x) = delete)
• Document the change in API with an appropriate migration path if there's a major release change. Ideally you would have these documented between minor releases and simply link to them at major change
• If possible keep the online api documentation for all the minor versions

Of course, you can have various variants on these depending on the specific change made

Since the original project was not setup with these in mind it is ok for the v2 major version change to be a bit more loose while converging to flow you feel more comfortable.

Thanks @LecrisUT for your detailed information. It may be difficult to follow the ideal way, but we will try to do the best.