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

fix!: remove unused or obsolete useragent checks #6355

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

rachel-fenichel
Copy link
Collaborator

@rachel-fenichel rachel-fenichel commented Aug 17, 2022

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Followup from #6336

Proposed Changes

Remove exported useragent booleans for chrome, webkit, ipod, and tablet.

Behavior Before Change

No change.

Behavior After Change

No change.

Reason for Changes

We weren't using these checks, and it's fragile for us to be maintaining and exporting them if we're not using them.

Test Coverage

Standard tests. We'll want to test on iOS for the release, but it should only impact sound loading anyway.

Documentation

Additional Information

Developers who were using these values should read the MDN page on browser detection using the user agent. If there is no way to detect what you're looking for without checking the user agent, follow the instructions on that page to find the most targeted possible solution.

@rachel-fenichel
Copy link
Collaborator Author

I'm not sure if this counts as a chore or a fix. It addresses a requested follow-up from #6336.

Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Not a breaking change because we were not re-exporting userAgent from the top level before.

I'm not sure this is correct. It looks like we're exporting it on master:

@rachel-fenichel rachel-fenichel added breaking change Used to mark a PR or issue that changes our public APIs. and removed type: cleanup labels Aug 17, 2022
@rachel-fenichel rachel-fenichel changed the title fix: remove unused or obsolete useragent checks fix!: remove unused or obsolete useragent checks Aug 17, 2022
@rachel-fenichel
Copy link
Collaborator Author

Good catch. Changed to be a breaking change.

Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

LGTM! Please update the PR description with info about what people should do if they're relying on these things.

@rachel-fenichel rachel-fenichel merged commit aff21b9 into google:develop Aug 17, 2022
@rachel-fenichel rachel-fenichel deleted the remove_useragents branch August 17, 2022 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. component: browser PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants