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.

Sign up for GitHub

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

Jump to bottom

Mark deprecated functionality with C++ attributes and resolve internal usage of deprecated functions #4551

Merged
merged 17 commits into from
Jul 29, 2024
Merged

Mark deprecated functionality with C++ attributes and resolve internal usage of deprecated functions #4551

merged 17 commits into from
Jul 29, 2024

Conversation

matejk
Copy link
Contributor

@matejk matejk commented May 8, 2024

Closes #4426.

@matejk matejk self-assigned this May 8, 2024
@matejk matejk force-pushed the 4426-deprecated-functionality branch 12 times, most recently from f2c5384 to bc4b50a Compare May 10, 2024 13:20
aleks-f
aleks-f requested changes May 12, 2024
View reviewed changes
Copy link
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

I am not sure if @deprecated is needed/used by PocoDoc, @obiltschnig would know

@matejk matejk force-pushed the 4426-deprecated-functionality branch from bc4b50a to 3999dcd Compare May 13, 2024 11:31
@matejk matejk force-pushed the 4426-deprecated-functionality branch 2 times, most recently from f63afd3 to da8037a Compare May 24, 2024 07:33
@matejk matejk force-pushed the 4426-deprecated-functionality branch from da8037a to fe5f2b5 Compare July 12, 2024 09:26
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 12, 2024
View reviewed changes
@matejk matejk requested review from obiltschnig and aleks-f July 15, 2024 10:04
@matejk
Copy link
Contributor Author

matejk commented Jul 15, 2024

Functionality that was marked as deprecated with comments is now marked as such with C++ attribute with a few exceptions:

Usage of deprecated functions and classes in Poco code is replaced with non-deprecated variants.

Unit tests for deprecated functionality are disabled by default and can be enabled with POCO_TEST_DEPRECATED.

Some C++ code modernisation was done in the code while resolving deprecations.

@aleks-f, @obiltschnig

aleks-f
aleks-f approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

Overall looks good, than you. I'm just thinking it would be good to have a single place to disarm warnings somehow.

Data/src/SQLChannel.cpp
@@ -235,6 +232,7 @@ void SQLChannel::run()
{
try
{
sleepTime = 100;
Copy link
Member

Choose a reason for hiding this comment

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

warning: likely to cause merge conflict with #4530

@matejk
Copy link
Contributor Author

matejk commented Jul 16, 2024 •
edited
Loading

Overall looks good, than you. I'm just thinking it would be good to have a single place to disarm warnings somehow.

Do you mean deprecation warnings that will be seen by the users of Poco? Do we want to disarm them?

@aleks-f
Copy link
Member

aleks-f commented Jul 16, 2024

Overall looks good, than you. I'm just thinking it would be good to have a single place to disarm warnings somehow.

Do you mean deprecation warnings that will be seen by the users of Poco? Do we want to disarm them?

I foresee some annoyance if we swamp the build with deprecation warnings. In any case, I think there should be an easy way to disable them.

matejk added 11 commits July 25, 2024 09:55
@matejk
fix(CMake): Variable BUILD_SHARED_LIBS must be defined properly to cr…
16002c1 
…eate valid binaries.
@matejk
enh: Code improvements done while resolving deprecated functionality (#…
e9b26ee 
…4426)
@matejk
Un-deprecate LocalDateTme (#4426)
c4e01e1
@matejk
enh(Poco): Replace usage of deprecated functionality with other funct…
5587305 
…ions/classes (#4426)
@matejk
chore(SSL): temporarily un-deprecate SSL-related functionality (#4426)
f8c0ce0
@matejk
chore(SSL): temporarily un-deprecate old MongoDB protocol functionali…
5d72939 
…ty (#4426)
@matejk
enh(Poco): Minor Hash improvements (#4426)
2e9cb49
@matejk
enh(Foundation): Compile deprecated hash tests only when POCO_TEST_DE…
f9fc941 
…PRECATED is enabled (#4426)
@matejk
enh(Net): Compile deprecated Socket::select functionality only when P…
0df7424 
…OCO_TEST_DEPRECATED is enabled (#4426)
@matejk
enh(Bonjour): Replace deprecated Socket::select with PollSet (#4426)
671c8a9
@matejk
enh(Poco): Introduce POCO_DEPRECATED macro to have the ability to dis…
ad8c34f 
…able deprecation warnings in applications (#4426)
@matejk matejk force-pushed the 4426-deprecated-functionality branch from 105597b to ad8c34f Compare July 25, 2024 08:46
@matejk
Copy link
Contributor Author

matejk commented Jul 25, 2024

Overall looks good, than you. I'm just thinking it would be good to have a single place to disarm warnings somehow.

Do you mean deprecation warnings that will be seen by the users of Poco? Do we want to disarm them?

I foresee some annoyance if we swamp the build with deprecation warnings. In any case, I think there should be an easy way to disable them.

Introduced a macro POCO_DEPRECATED instead of using [[deprecated]] directly. An application can define POCO_SILENCE_DEPRECATED to have POCO_DEPRECATED defined to an empty define.

IMO the branch is ready to be merged to main.

@obiltschnig, @aleks-f, any objections?

@matejk matejk merged commit 91bea7c into main Jul 29, 2024
42 checks passed
matejk added a commit that referenced this pull request Jul 29, 2024
@matejk @aleks-f
enh(Poco): Mark deprecated functionality with C++ attributes and reso…
f24547c 
…lve internal usage of deprecated functions (#4551)

* enh(poco): Replace deprecated comments with C++ deprecated attribute.

* enh(Poco): Replace some deprecated functionality in Poco sources. (#4426)

* enh(Poco): Replace more deprecated functionality in Poco sources. (#4426)

* fix(CMake): Variable BUILD_SHARED_LIBS must be defined properly to create valid binaries.

* enh: Code improvements done while resolving deprecated functionality (#4426)

* Un-deprecate LocalDateTme (#4426)

* enh(Poco): Replace usage of deprecated functionality with other functions/classes (#4426)

* chore(SSL): temporarily un-deprecate SSL-related functionality (#4426)

* chore(SSL): temporarily un-deprecate old MongoDB protocol functionality (#4426)

* enh(Poco): Minor Hash improvements (#4426)

* enh(Foundation): Compile deprecated hash tests only when POCO_TEST_DEPRECATED is enabled (#4426)

* enh(Net): Compile deprecated Socket::select functionality only when POCO_TEST_DEPRECATED is enabled (#4426)

* enh(Bonjour): Replace deprecated Socket::select with PollSet (#4426)

* enh(Poco): Introduce POCO_DEPRECATED macro to have the ability to disable deprecation warnings in applications (#4426)

* test(ODBC): add few asserts to testStoredProcedureDynamicVar

* fix(ODBC): rename DynamicAny -> DynamicVar in tests

* fix(ODBC): make Dignostics static members inline to prevent explicit instantiation warnings on windows

---------

Co-authored-by: Alex Fabijanic <alex@pocoproject.org>
@matejk matejk deleted the 4426-deprecated-functionality branch July 29, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aleks-f aleks-f aleks-f approved these changes

@obiltschnig obiltschnig Awaiting requested review from obiltschnig

Assignees

@matejk matejk

Labels
None yet
Projects
1.14
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove deprecated code
2 participants
@matejk @aleks-f