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

Record invalid items as tree problems #58

Closed
linabutler opened this issue Mar 2, 2020 · 7 comments · Fixed by #59
Closed

Record invalid items as tree problems #58

linabutler opened this issue Mar 2, 2020 · 7 comments · Fixed by #59
Assignees

Comments

@linabutler
Copy link
Contributor

linabutler commented Mar 2, 2020

Currently, we use ProblemCounts to report tree structure problems—missing or invalid parents and children, and the like. However, I wonder if it also makes sense to report how many invalid items we have. It's what we use to flag malformed URLs, for remote and now local items. This would help us see how frequently we encounter and flag invalid URLs in telemetry.

As part of fixing this bug, in Dogear, we'll want to:

  • Add a new variant to Problem for recording invalid items, and their GUIDs. We'll need to change other types like ProblemSummary, too; the compiler should help us out here.
  • When we iterate over all tree entries in this loop, note a problem if entry.item.validity == Validity::Replace.
  • Add a new field to ProblemCounts for summing these up.

And, finally, in Firefox, we'll need to bubble this count up in the property bag here, then change this method to extract those problems from the bag, and report them in validation telemetry. That work is a bit more involved, and we can get a separate Bugzilla bug on file once this one is fixed.

@linabutler
Copy link
Contributor Author

@lougeniaC64 How do you feel about taking this bug? 😄

@lougeniaC64
Copy link
Contributor

@linacambridge 😬 I'm up for it. I would like to get more familiar with this logic. Just an FYI though, I'm working on this remerge ticket first so I may not get to it right away. Let me know if that's okay.

@linabutler
Copy link
Contributor Author

Oh, absolutely, this is definitely lower priority than the Remerge work. Thank you!! 🎉💯

@linabutler
Copy link
Contributor Author

And, finally, in Firefox, we'll need to bubble this count up in the property bag here, then change this method to extract those problems from the bag, and report them in validation telemetry. That work is a bit more involved, and we can get a separate Bugzilla bug on file once this one is fixed.

I need to file this now! 😁

@lougeniaC64, do you have a crates.io account? If you'd like to make one, I'll add you as an owner so you can publish Dogear releases, too!

@linabutler
Copy link
Contributor Author

I'll add you as an owner so you can publish Dogear releases, too!

@mhammond, too! 😄

@lougeniaC64
Copy link
Contributor

And, finally, in Firefox, we'll need to bubble this count up in the property bag here, then change this method to extract those problems from the bag, and report them in validation telemetry. That work is a bit more involved, and we can get a separate Bugzilla bug on file once this one is fixed.

I need to file this now! 😁

@lougeniaC64, do you have a crates.io account? If you'd like to make one, I'll add you as an owner so you can publish Dogear releases, too!

@linacambridge Just created one with my github credentials.

@linabutler
Copy link
Contributor Author

Awesome, thanks @lougeniaC64! I'm not sure if it notifies you by email, but I just send you an owner invite.

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 a pull request may close this issue.

2 participants