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 TooManySeeks, instead of removing #618

Merged
merged 2 commits into from Jan 27, 2021

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Jan 26, 2021

Recently (in #611) I removed the TooManySeeks exception, which has been unused in the code since 2015.

But, just because we don't use it doesn't mean that nobody ELSE uses it. So, in case any of the library's consumers are still catching TooManySeeks (as we no longer do), to avoid breaking their code this PR restores the exception. However, both the class and the constructor are marked as deprecated, causing both g++ and clang++ to emit a warning if it's used:

g++:

[ 60%] Building CXX object src/CMakeFiles/openshot.dir/Qt/PlayerPrivate.cpp.o
src/Qt/PlayerPrivate.cpp: In member function ‘std::shared_ptr<openshot::Frame> openshot::PlayerPrivate::getFrame()’:
src/Qt/PlayerPrivate.cpp:178:35: warning: ‘TooManySeeks’ is deprecated: The library no longer throws this exception. It will be removed in a future release. [-Wdeprecated-declarations]
  178 |     } catch (const TooManySeeks & e) {
      |                                   ^
In file included from src/Qt/PlayerPrivate.cpp:33:
src/Exceptions.h:375:2: note: declared here
  375 |  TooManySeeks : public ExceptionBase
      |  ^~~~~~~~~~~~

clang++:

[ 61%] Building CXX object src/CMakeFiles/openshot.dir/Qt/PlayerPrivate.cpp.o
src/Qt/PlayerPrivate.cpp:178:20: warning: 'TooManySeeks' is deprecated: The library no longer throws this exception. It will be removed in a future release. [-Wdeprecated-declarations]
    } catch (const TooManySeeks & e) {
                   ^
src/Exceptions.h:373:18: note: 'TooManySeeks' has been explicitly marked deprecated here
        __attribute__ ((deprecated (TMS_DEP_MSG) ))
                        ^
1 warning generated.

TooManySeeks is also excluded from the SWIG bindings entirely using #ifndef SWIG.

A separate commit in this PR moves most (all?) #include "Exceptions.h" lines from the header files to their corresponding .cpp files, to reduce unnecessary recompilations and header bloat.

To avoid API changes, we should phase the deprecation of TooManySeeks
in case external callers are catching it. But it remains unused in
the code, and should be considered deprecated. Add a deprecation
message instead.

Exceptions: Exclude TooManySeeks from SWIG
@codecov
Copy link

codecov bot commented Jan 26, 2021

Codecov Report

Merging #618 (2f3615f) into develop (2563c3c) will not change coverage.
The diff coverage is 14.28%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #618   +/-   ##
========================================
  Coverage    52.24%   52.24%           
========================================
  Files          129      129           
  Lines        10774    10774           
========================================
  Hits          5629     5629           
  Misses        5145     5145           
Impacted Files Coverage Δ
src/AudioReaderSource.cpp 0.00% <ø> (ø)
src/CacheBase.h 75.00% <ø> (ø)
src/CacheDisk.cpp 71.69% <ø> (ø)
src/CacheMemory.cpp 90.07% <ø> (ø)
src/CacheMemory.h 0.00% <ø> (ø)
src/ChunkReader.cpp 0.00% <ø> (ø)
src/ChunkWriter.cpp 0.00% <ø> (ø)
src/ChunkWriter.h 0.00% <ø> (ø)
src/Clip.cpp 43.74% <ø> (ø)
src/ClipBase.h 80.76% <ø> (ø)
... and 57 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2563c3c...2f3615f. Read the comment docs.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Jan 27, 2021

Merging this, as it's what I should've done in the first place. (It just took me a bit more time to figure out the right syntax for deprecating a class, and to ensure that it worked with both GCC and Clang.)

@ferdnyc ferdnyc merged commit b5c3d0b into OpenShot:develop Jan 27, 2021
@ferdnyc ferdnyc deleted the deprecate-TooManySeeks branch January 27, 2021 03:59
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

1 participant