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

create a bugscore report #2331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sylvestre
Copy link
Contributor

Comment on lines +70 to +71
<script>
document.addEventListener('DOMContentLoaded', function () {
Copy link
Member

Choose a reason for hiding this comment

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

I do not think we could use js in emails.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, we can't :)
but this doesn't matter much. I will export the html on a website

Copy link
Member

Choose a reason for hiding this comment

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

We could make the bot export the HTML page. Some of the simple options could be a GCS bucket or GitHub pages (based on the same repo or a new repo). A subdomain could point to that page (e.g., reports.moz.tools).

We could also do the same for other rules so that the web pages will have the latest updated data.

Copy link
Member

@suhaibmujahid suhaibmujahid left a comment

Choose a reason for hiding this comment

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

Thank you, Sylvestre! Looks interesting! Could you please fix the formatting to make the CI happy?

Comment on lines +12 to +16
self.ndups = self.get_config("number_dups")
self.votes = self.get_config("number_votes")
self.cc = self.get_config("number_cc")
self.see_also = self.get_config("number_see_also")
self.comments = self.get_config("number_comments")
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use the constructor parameters to configure the rules instead of the config file (see #1437)?

Alternatively, you could assign the values directly, just like the weights, without using the configuration file.

Comment on lines +12 to +16
self.ndups = self.get_config("number_dups")
self.votes = self.get_config("number_votes")
self.cc = self.get_config("number_cc")
self.see_also = self.get_config("number_see_also")
self.comments = self.get_config("number_comments")
Copy link
Member

Choose a reason for hiding this comment

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

We could unify the variable naming in a descriptive way (e.g., min_comments or num_comments_threshold)

Comment on lines +12 to +16
self.ndups = self.get_config("number_dups")
self.votes = self.get_config("number_votes")
self.cc = self.get_config("number_cc")
self.see_also = self.get_config("number_see_also")
self.comments = self.get_config("number_comments")
Copy link
Member

Choose a reason for hiding this comment

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

nit: It could be useful to add a comment to clarify that these thresholds are to highlight the values in the table, not to be used in the query.

"f3": "OP",
"f4": "dupe_count",
"o4": "greaterthaneq",
"v4": "5",
Copy link
Member

Choose a reason for hiding this comment

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

nit: Declaring these thresholds in the constructor after the highlighting thresholds will help distinguish between the two types of thresholds.

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