Does not work with Github enterprise

[sahilsingla90]

No description provided.

[sanemat]

Good catch!

I don't have an idea, yet. Chrome extension requires target url.

do-not-merge-wip-for-github/app/manifest.json

Lines 22 to 24 in df5fc2c

	"matches": [
	"https://github.com/*/*/pull/*"
	],

            "matches": [
                "https://github.com/*/*/pull/*"
            ],

matches: ["*"] is "overkill", I think.

[sahilsingla90]

I think the solution is straight forward. Make the github domain to be specified in the options with default as github.com

[sanemat]

I prefer a custom build like
npm run build -- --matches=https://example.com/* --matches=https://test.com/*.
This will append matches to manifest.json.

"matches": [
    "https://github.com/*/*/pull/*",
    "https://example.com/*",
    "https://test.com/*"
],

This keeps matches simple and secure.

[sahilsingla90]

That does make sense, but that would also require additional step for the company to make the custom build and provide it to all the employees, besides explaining how to install a custom built chrome extension which just seems unnecessary.
Thus from the viewpoint of usability (and getting the most users to use your extension) makes sense to have it as an option, in my opinion.

[mckelvin]

+1 for this

[Justineo]

Theoretically you can set <all_urls> as an optional permission in manifest and dynamically request domain permissions according to those user set in extension options. But Chrome's implementation of optional permission is broken (see here) so AFAIK the only way to do this is to request domain permission for <all_urls> and take care of the injection by yourself.

[liangway]

+1 for this too