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

Bug 1878140 - Make sendBeacon fallback the default uploader. #1876

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

perrymcmanis144
Copy link
Contributor

@perrymcmanis144 perrymcmanis144 commented Feb 12, 2024

Relies on existing tests.

Prior changes did not include doc changes. @rosahbruno what doc changes would you like to see, if any?

Pull Request checklist

  • Quality: Make sure this PR builds and runs cleanly.
    • Inside the glean/ folder, run:
      • npm run test Runs all tests
      • npm run lint Runs all linters
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work

@rosahbruno
Copy link
Contributor

Relies on existing tests.

We could probably create a test where we set something like global.navigator.sendBeacon = undefined and ensure that the fetch call is made. I don't think this is a make or break, but just something we could do. Similar tests exist in sendbeacon_fallback_uploader.spec.ts.

what doc changes would you like to see, if any?

We are currently making a list of things that we need to add for the new Glean.js docs. I can add this to the list. I don't think we need anything specific for this now. I see in the SDK book there is a section for custom uploaders where we could maybe add a note about this - but I think we can just address it better in the new docs.

CHANGELOG.md Outdated Show resolved Hide resolved
@perrymcmanis144 perrymcmanis144 force-pushed the sendbeacon_fallback branch 4 times, most recently from c374edd to fde3333 Compare February 15, 2024 19:02
@perrymcmanis144 perrymcmanis144 marked this pull request as ready for review February 15, 2024 19:07
@auto-assign auto-assign bot requested a review from Dexterp37 February 15, 2024 19:07
Copy link
Contributor

@rosahbruno rosahbruno left a comment

Choose a reason for hiding this comment

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

LGTM, Nice work on this! 💯

@rosahbruno rosahbruno merged commit 702c810 into main Feb 15, 2024
5 of 6 checks passed
@rosahbruno rosahbruno deleted the sendbeacon_fallback branch February 15, 2024 21:12
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

2 participants