GitHub enterprise support
arguswaikhom opened this issue · 11 comments
Can we have GitHub enterprise support on this extension?
e.g. github.x.com
I think making and option to add site access would do the job.
Hey @arguswaikhom . Sorry, I'm very late to this issue.
Yeah, this sounds very reasonable. Do you have a link to an public enterprise repo I could use for testing (or do you know if that's even possible?).
Hello @Claudiohbsantos, I couldn't find a publicly accessible enterprise URL. It is usually required to use the company's VPN to access them.
Here is an example:
Turns out this is actually trickier than I would have imagined.
match patterns only support matching a host and it's subdomains, the host with a specified subdomain or a blanket all hosts. In this case we'd need to have a match on the "enterprise" domain (cerner in this case). Since we can't predict what enterprise domains might be needed, in order to support this we would need to include all hosts in the permissions and pattern match the url alone.
I want to take a better look to see if there's any way to avoid asking for such wide permissions before making this move. Ideally I can find a way to make all websites "opt-in" so we're not just getting permissions to read and write to all websites (which is something I don't think this extension should do)
webext-dynamic-content-scripts seems like maybe a good solution for this, and other extensions that need to support GHE seem to be using it to avoid requesting all host permissions. There seems to be ongoing work to support Manifest V3 so I'll just keep an eye on how that's going. Once that is released I'll give it a shot and if it works well I'll try to use it to support Github Enterprise
MV3 should now be supported. Let me know how it goes. The full integration guide is in https://github.com/fregante/webext-dynamic-content-scripts/blob/main/how-to-add-github-enterprise-support-to-web-extensions.md
@Claudiohbsantos are you still working on this? I can also make a PR to add support for this.
This was fixed in #85, it was merged in the new fork and then the fork was merged back here. You can close this issue now.
idk what u did but it wants all websites by default now. I thought it was opt in
idk what u did but it wants all websites by default now. I thought it was opt in
I think this is related to this comment on the other issue #99 (comment)
I know there is a way to toggle it by having a setting in the extension's config files instead of on install
nvm there should be a way to do it (btw the extension i used as example is MAL-Sync