DaemonEngine / Daemon

The Dæmon game engine. With some bits of ioq3 and XreaL.
https://unvanquished.net
BSD 3-Clause "New" or "Revised" License
306 stars 60 forks source link

mark deprecated things with ` [[deprecated]]` #583

Closed ghost closed 2 years ago

ghost commented 2 years ago

And for stuff to deprecate in future times, do something along the lines of:

#if VER_MAJOR == 0 && VER_MINOR <= 53
#define DEPRECATED( version ) [[deprecated]]
#else
#define DEPRECATED( version )
#endif
ghost commented 2 years ago

Note: I'm talking about things like this. Also, note that saying "deprecated, use foo instead" is not enough, it would be better to show a replacement example.

slipher commented 2 years ago

I don't think deprecated annotations are useful unless the maintainers of the deprecated function and its users are different groups of people.

If we have a deprecation warning and it is treated as an error, then we are forced to immediately migrate everything, so the function could just be deleted instead of deprecated. If we have a deprecation warning and it is not treated as an error, then we have a lot of warning spam which makes it hard to see other useful warnings.

necessarily-equal commented 2 years ago

I think this is the kind of stuff that we may not want to care about before daemon gets several games running on it and we need some form of ABI.

This sounds like it would be a waste of resources at the moment. Definitely not a large waste of resources though, but I fail to see the point

ghost commented 2 years ago

SImply to help removing the actually deprecated stuff. In the case I'm talking about, it would help making code clearer... if there is some documentation added as to how to remove the deprecated stuff, ofc, otherwise... well, things are hard to guess why deprecate or what to do to fix it.

OTOH... I'm working with entities those days, and there, shitload of things are said "deprecated", yet, they are used by tremulous maps, which are more than 100, while unvanquished really have less than 10 maps (and by deprecating things, bugs were introduced, even!), so I can understand your point.

slipher commented 2 years ago

Warnings that aren't included in the CI's -Werror are generally harmful to developer productivity. By producing a lot of terminal output, they make it harder to see the warnings from newly written code which are far more important than ones from existing code. Because the new warnings are more likely to indicate actual bugs, and because some of the new ones may be ones that we have in the CI, meaning the developer is required to fix them before merging the code.

ghost commented 2 years ago

Agree.