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

Deprecate functions and constants more gently #4952

Merged
merged 10 commits into from
Jan 25, 2019
Merged

Conversation

ethomson
Copy link
Member

Provide a new kindler, gentler deprecation policy for the upcoming release.

  1. Move deprecated bits into their own header file deprecated.h.
  2. Remove the GIT_DEPRECATED attribute from the deprecated bits. This will allow users to more gently move into the new functions at their leisure. We have no hurry in removing the old functions or values, so let's not rush things.
  3. Allow users to opt-in to a harder deprecation, by defining GIT_DEPRECATE_HARD to completely remove the deprecated functions.
  4. Enable GIT_DEPRECATE_HARD ourselves to ensure that we don't use them in the library code, tests, or examples.

I've kept the GIT_DEPRECATED macro so that we can mark something as properly deprecated, using the compiler attributes, in the future if we desire.

@ethomson ethomson force-pushed the ethomson/deprecation branch 5 times, most recently from 85bfd71 to ce2f006 Compare January 23, 2019 13:39
@ethomson
Copy link
Member Author

OK, here's a proposal for a "softer" deprecation; instead of marking things as deprecated with an attribute, we simply stuff them into their own file. The exception being sys/streams.h; since sys header files aren't included by default (from git2.h), we want to keep the deprecated functions there.

This introduces GIT_DEPRECATE_HARD, which you can define if you want to avoid deprecated bits entirely.

ethomson referenced this pull request Jan 24, 2019
The C standard does not specify whether an enum is a signed or unsigned
type.  Obviously, any enum that includes negative values _must_ be
signed, but if all values are positive then the compiler is free to
choose signed or unsigned.

Thus, by changing the type signatures to `git_object_t` and declaring
the old `GIT_OBJ_` values as a signed or unsigned int, we risk a
mismatch between what the compiler has chosen for a `git_object_t`'s
type and our type declaration.

Thus, we declare the deprecated values as the enum instead of guessing.
Copy link
Member

@pks-t pks-t left a comment

Choose a reason for hiding this comment

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

I'm perfectly happy with this approach, so I'm in :)

src/common.h Outdated
@@ -76,6 +76,7 @@

#include "git2/types.h"
#include "git2/errors.h"
#include "git2/deprecated.h"
Copy link
Member

Choose a reason for hiding this comment

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

This include shouldn't be necessary in the end, right? None of our internal code is supposed to use deprecated functions or constants. With the exception of test code, I guess

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, good call.

@ethomson ethomson force-pushed the ethomson/deprecation branch 2 times, most recently from a1dfa48 to 972d1dd Compare January 25, 2019 08:49
`git_stream_register_tls` is now deprecated; mark it in an if guard with
the deprecation.

This should not be included in `deprecated.h` since it is an uncommonly
used `sys` header file.
Add `@deprecated` to the functions that are, so that they'll appear that
way in docurium.
Avoid the deprecated `git_stream_cb` typedef since we want to compile
the library without deprecated functions or types.  Instead, we can
unroll the alias to its actual type.
Move the deprecated stream tests into their own compilation unit.  This
will allow us to disable any preprocessor directives that apply to
deprecation just for these tests (eg, disabling `GIT_DEPRECATED_HARD`).
Users can define `GIT_DEPRECATE_HARD` if they want to remove all
functions that we've "softly" deprecated.
Ensure that we do not use any deprecated functions in the library
source, test code or examples.
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.

None yet

2 participants