Bug 1640575 - Move "debug view tagging" to Rust #998
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brizental
commented
Jun 23, 2020
brizental
commented
Jun 23, 2020
Dexterp37
requested changes
Jun 23, 2020
There was a problem hiding this comment.
Thanks @brizental !I took an high level look first, there's two major request I have:
- let's defer updating the serialization format;
- let's not provide a public API for this
What do you think?
brizental
force-pushed
the
1640575-debug-tag
branch
2 times, most recently
from
fd5f5b1 to
c79d79e
Compare
mdboom
suggested changes
Jun 23, 2020
brizental
force-pushed
the
1640575-debug-tag
branch
from
c79d79e to
fa9302c
Compare
brizental
force-pushed
the
1640575-debug-tag
branch
from
fa9302c to
769bd4c
Compare
Dexterp37
approved these changes
Jun 24, 2020
glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt
Show resolved
Hide resolved
BONUS: remove some duplicated code from tests
brizental
force-pushed
the
1640575-debug-tag
branch
from
769bd4c to
24144f8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes Bug 1640575.
As a bonus also fixes (or at least partially fixes) Bug 1605097, because persisting the tag was the best way I found to move it to Rust.Not anymore, this was defered to a later PR. Because the tag was not being persisted any longer, I had to drop the commit where I removed the debug view tag management from python in favour of the rust implementation. Since python instantiates a standalone upload manager to do the upload, without persisting the tag we don't pck up on it during upload. When I work on the PR to persist the headers I can bring the python commit back.I realized after the fact that this could be broken down into smaller PRs, which can stil be done if you prefer since this is already broken down into coherent commits. Please let me know and I'll close this if necessary.