miketaylr / web-bugs-issue-tagger

Tiny app that listens for webhook issue events and adds tags to issues as needed.
0 stars 0 forks source link

Don't add browser version to label #7

Closed miketaylr closed 9 years ago

miketaylr commented 9 years ago

Right now, set_label will grab everything in the foo from <!-- @browser: foo -->, append browser- to it and set that as the label. So if you have @browser: foo nightly 41 you get browser-foo-nightly-41 which isn't great.

miketaylr commented 9 years ago

We populate @browser from Browser / Version field, which maybe isn't the best idea.

Maybe we should run ua_parser on the UA string and grab the family name instead (possibly matching against a whitelist)? Right now if you put in "Cool Guy Foo" it'll create a browser-cool-guy-foo label which is spam.

miketaylr commented 9 years ago

Thoughts @karlcow?

miketaylr commented 9 years ago

To clarify:

Maybe we should run ua_parser on the UA string and grab the family name instead (possibly matching against a whitelist)?

That's how we get 1/2 of the "Browser / Version" field. So this would be doing the same and just ignoring the version.

karlcow commented 9 years ago

@miketaylr

match_list = re.search(r'<!--\s@(\w+):\s([^\d]+?)\s\d+.\d+\s-->', body)

This is working too.

capture d ecran 2015-07-30 a 07 19 45

karlcow commented 9 years ago

ok my bad it was failing with things like Opera Mobile 31.0.1890. Updated the regex

match_list = re.search(r'<!--\s@(\w+):\s([^\d]+?)\s[\d\.]+\s-->', body)
karlcow commented 9 years ago

Additional possibilites.

miketaylr commented 9 years ago

@karlcow the updated regex looks good, thanks!. ^_^ If you want to commit that, I can push to Heroku. Or I can make a commit in the morning.

karlcow commented 9 years ago

@miketaylr Done.