Implement support for other Jenkins hosts

[akoeplinger]

Adds an initial attempt at allowing the extension to support non-dotnet Jenkins hosts.

To avoid hardcoding the URLs of other hosts we need to add an "optional_permissions" key to manifest.json. By claiming we need http://*/* we can request permissions for specific URLs on demand.

Unfortunately, the chrome.permissions.request API requires being invoked from an explicit user action like a button click.

To add to that we can't even access it from a button in the content.js script, it only seems to work in options.js. Googling revealed a few workarounds involving an iframe that sends messages to options.js, but that seems too involved for now.

Instead, I opted for the simplest solution of providing a text box in options.html where you can simply paste a new URL, press the button and Chrome will ask for confirmation.

Fixes #57

Note: I also dropped ajax.googleapis.com since it doesn't seem to be used.

[dpoeschl]

I like the idea of just building this into the options page for now.

I have some local changes that enables this on button/link clicks from content.js. You have to message pass over to background.js, from a user action context, but then it seems to work. But then I got bogged down in the details of making the UI on the actual PR page sensical/pretty. I'm very happy to use this approach as a fix in the mean time. I'll try it out soon.

[dpoeschl]

@akoeplinger I can post my in-progress branch with those working button-based changes if you're interested, but your approach will unblock people who care about this enough.

[akoeplinger]

@dpoeschl I'd appreciate if you could push your branch somewhere so I can play around with it :)

When do you plan on making a new release? I'd like to show this extension to a few members of my team at Xamarin.

[dpoeschl]

I'll push my branch when I get home (it's on that machine currently).

I was planning for a release this coming (long) weekend. With this change, we'd have a couple small bug fixes, the addition of non-"dotnet" repository support, and I have a code review feature I've been working on locally (aggregating :+1:/LGTM/etc. comments into a list at the top of the PR, with a few other things to go along with it). Does that sound like an okay timeline?

[akoeplinger]

Sounds awesome :sparkles: Edit(dpoeschl): :-1: Test Signoff

[dpoeschl]

@akoeplinger Checked in the PR-side test server access stuff, and I like having both parts (being able to see access in the options page & manually add there if desired, and also being able to click something once the access failure has just occurred). Here's how it works:

From the content script, on a user action: https://github.com/dpoeschl/StashPop/blob/master/content.js#L423 On the background script side, do the actual request: https://github.com/dpoeschl/StashPop/blob/master/background.js#L56

End result:

[dpoeschl]

Clicking "Allow" also auto invokes the "Reload Jenkins data" thing, so the build info shows up right away.

Edit: Adding :+1: here as a test

[akoeplinger]

@dpoeschl this is amazing :heart_eyes: can't imagine using GH without this extension anymore :+1:

One minor nit: the "Grant StashPop access" links should be removed when the access was granted from the popup.

[dpoeschl]

Yeah, I skipped that detail in the process of trying to get a release together today. It's on my optional-to-implement-before-releasing-tonight list. It could also be better supported in the "Show Jenkins Details" links in the PR list, but I figure it's a one-time operation so minor shortcomings shouldn't hold up release. I hope to publish in the next 3-4 hours after some more testing / tweaking. :smile:

If you're running latest, please feel free to share any gut reactions to the Code Reviews thing.

[dpoeschl]

Also, crazy corner case -- I have to figure out which "Grant StashPop access" (better text ideas welcome) links to remove, because I believe it's possible to run tests from multiple test servers?

[akoeplinger]

... but I figure it's a one-time operation so minor shortcomings shouldn't hold up release

I agree, it's just a minor polish thing.

... I believe it's possible to run tests from multiple test servers?

Yeah, good point. Should be possible by looking at the URLs of the tests though.

... gut reactions to the Code Reviews thing.

I like it! One thing that might be useful is to only show the indicators for comments from those with push access to the repo and not everyone, though that's likely harder to implement.

[dpoeschl]

One thing that might be useful is to only show the indicators for comments from those with push access to the repo and not everyone, though that's likely harder to implement.

Yeah. For the signoff aggregation at the top, I'm grabbing people's "role" ("Owner" / "Contributor" / are there more?) and putting it in the tooltip. I can prioritize the ones with certain roles, and I'm looking for a way of highlighting those ones in the list. Hopefully that will help for that part.

For the comment header background color thing, maybe I just won't color the background of commenters without roles, or find a way to be much more subdued about it. I'll play around with that idea a bit.

Edit for testing: :-1: :+1: Test Signoff

[dpoeschl]

Playing around:

[dpoeschl]

@akoeplinger New version released! Thanks for all your help and feedback -- keep it coming! :smile: https://twitter.com/StashPopExt/status/689495626925477888

[akoeplinger]

Thanks! I already sent out a link to all my colleagues :smile: