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

Remove old CEF ifdefs #302

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Remove old CEF ifdefs #302

merged 1 commit into from
Nov 29, 2021

Conversation

gxalpha
Copy link
Member

@gxalpha gxalpha commented Jul 12, 2021

Description

Removes ifdefs that only benefit anyone using a CEF version older than 3770.
Since 3770 is the oldest version of CEF supported and shipped, their content was dead code.

This dead code got removed, none of the current functionality on any CEF >= 3770 should be effected.

Motivation and Context

Stumbled across some of them while working on something else, and noticed they weren't needed.

How Has This Been Tested?

Compiled on macOS.
This did require me to rebuild the cmake project and clear Xcode caches since Xcode has some scripts that assumed the removed header was still there, but that isn't project-related.

I unfortunately can't test Windows.

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM
Copy link
Member

As it stands, I'd personally prefer to make this change once we've actually moved away from 3770. Essentially, internally "support" one version below what's currently included, just in case things go haywire and we need to revert the CEF version for some reason.

@gxalpha
Copy link
Member Author

gxalpha commented Jul 13, 2021

Should I leave this PR open then and just wait?

@WizardCM
Copy link
Member

Yep, leave it open and we'll review it closer to v28.

@WizardCM WizardCM added this to the OBS Studio 28.0 milestone Jul 13, 2021
@pkviet
Copy link
Member

pkviet commented Nov 7, 2021

Lgtm

@WizardCM
Copy link
Member

After merging #323 this now has merge conflicts. Once they're solved, I'll merge this.

@WizardCM
Copy link
Member

Ah, sorry I didn't catch this earlier - I would like to keep the ENABLE_WASHIDDEN option, as the goal (which I realise may not be realistic) is to eventually switch back to the previous behaviour.

Removes ifdefs effecting CEF versions older than 3770, since that is the
oldest version supported and used by OBS (as of 27.1).
@WizardCM WizardCM merged commit 618d47e into obsproject:master Nov 29, 2021
@gxalpha gxalpha deleted the old-cef-ifdefs branch November 29, 2021 17:20
@RytoEX RytoEX removed this from the OBS Studio 28.0 milestone Nov 29, 2021
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.

4 participants