8000 GitHub Enterprise integration by jbjonesjr · Pull Request #5 · LeanKit/LeanKit.IntegrationService · GitHub
[go: up one dir, main page]

Skip to content

GitHub Enterprise integration#5

Open
jbjonesjr wants to merge 4 commits intoLeanKit:masterfrom
jbjonesjr:github-enterprise-auth
Open

GitHub Enterprise integration#5
jbjonesjr wants to merge 4 commits intoLeanKit:masterfrom
jbjonesjr:github-enterprise-auth

Conversation

@jbjonesjr
Copy link

🚧 Work in progress 🚧
NOTE: I haven't written C# in 10+ years, so i'm starting with pretty close pseudo-code, and i'll figure out proper C# bits later.

Tasking

  • update README.md
  • update https://support.leankit.com/ documentation : @reverentgeek
  • Build config ui for github enterprise host, post
  • update github integration code to switch on enterprise vs .com, and use the correct host/port
  • github api url needs to be a variable so that it can handle the differences in url in .com and GitHub Enterprise
  • update code so the host is expected to have repo owner or organization as well
  • centralize this code (In GitHubConnection.cs ❓ ) to abstract the system type from the actual code

@jbjonesjr
Copy link
Author

closes #4

github integration is now updated as of 2016. update copyright date.
@jbjonesjr
Copy link
Author

@reverentgeek signing you up for updating leankit integration support docs. if only it was gh-pages....

- switch github.com vs github enterprise based on host configuration
- abstract logic into helper function
- switch rest api url based on enterprise or .com
- start working on whitespace/indention issues
@jbjonesjr
Copy link
Author

Why does GitHubIssuesSubSystem and GitHubPullsSubsystem not use GitHubConnection to access GitHub? Seems redundant, but maybe there's a reason...?

@reverentgeek
Copy link
Contributor

The current assumption in the Integration Service is the Host input field is the owner or organization name, and not the full URL. This value is used as part of the URL path when making API requests.

To support both github.com and Enterprise customers, I think we'll have to change the expected input to be either github.com/{owner} or {enterprise-url} and parse out owner in the case of the former.

When making API requests to Enterprise, I understand the api path starts with http(s)://hostname/api/v3/. Do Enterprise api requests have a concept of owner like standard GitHub API requests? E.g. GET /repos/:owner/:repo/issues is what we currently use to get a list of issues.

You're right, GitHubIssuesSubsystem and GitHubPullsSubsystem could probably be refactored to use GitHubConnection. That was probably on our list at one time, but never got around to it.

@jbjonesjr
Copy link
Author

good details.

The current assumption in the Integration Service is the Host input field is the owner or organization name, and not the full URL. This value is used as part of the URL path when making API requests.

Ahh, that makes sense. I couldn't understand what that line in the docs was trying to say. Actually enterprise will work the same way as github.com (in the sense of https://{enterprise_fqdn}/{owner} The Enteprise API is a superset of the GitHub.com API, so the calls and syntax is all the same.

I'm not sure what sort of templating engine is being used for the web-frontend, but would it be possible to extend with additional form fields or cascading selection options? (ie, ask for a repo owner/organization seperately from a github url)?

You're right, GitHubIssuesSubsystem and GitHubPullsSubsystem could probably be refactored to use GitHubConnection. That was probably on our list at one time, but never got around to it.

Just wasn't sure if they was a specific reason for this that I wasn't understanding. 👍

Docs: https://support.leankit.com/hc/en-us/articles/204413743-LeanKit-Integration-Service-Overview-Guide#configuration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0