[GitHub] [openwhisk-catalog] style95 commented on a change in pull request #321: Enhance github webhook endpoint to allow github enterprise api usage

[GitBox <gi...@apache.org>]

style95 commented on a change in pull request #321:
URL: https://github.com/apache/openwhisk-catalog/pull/321#discussion_r635677193



##########
File path: packages/github/webhook.js
##########
@@ -52,7 +59,7 @@ function main(params) {
   var whiskCallbackUrl = urlHost.protocol + '//' + process.env.__OW_API_KEY + "@" + urlHost.host + '/api/v1/namespaces/' + encodeURIComponent(triggerName[1]) + '/triggers/' + encodeURIComponent(triggerName[2]);
 
   // The URL to create the webhook on Github
-  var registrationEndpoint = 'https://api.github.com/repos/' + (organization ? organization : username) + '/' + repository + '/hooks';
+  var registrationEndpoint = 'https://'+ githubApi +'/repos/' + (organization ? organization : username) + '/' + repository + '/hooks';

Review comment:
       How about including a protocol into the variable too?
   In some cases, the protocol might need to be configurable.
   

##########
File path: packages/github/README.md
##########
@@ -38,6 +38,7 @@ The `/whisk.system/github/webhook` feed configures a service to fire a trigger w
 - `repository`: The GitHub repository.
 - `accessToken`: Your GitHub personal access token.
 - `events`: The [GitHub event type](https://developer.github.com/v3/activity/events/types/) of interest.
+- `githubApi`: The GitHub api endpoint. Default value is 'api.github.com'. 

Review comment:
       I think `endpoint` or something like `baseUrl` would be more intuitive.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org