Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement scanning custom GitHub Enterprise Server instance #126

Closed

Conversation

seqre
Copy link
Contributor

@seqre seqre commented Feb 11, 2024

Unfortunately, I do not have access to any GitHub Enterprise Server instance, so I couldn't test it out. It's a relatively simple change, so it should work.

I'm open to any suggestions for improvement.

@seqre seqre force-pushed the github-instance-scanning branch 2 times, most recently from 55879ce to d7eb987 Compare February 11, 2024 03:23
Copy link
Collaborator

@bradlarsen bradlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this option can be renamed to be more general? And also more cohesive with the existing options.

It would be nice if there were additional tests to exercise this, but I also don't have easy access to a GHES instance for testing.

crates/noseyparker-cli/src/args.rs Outdated Show resolved Hide resolved
crates/noseyparker-cli/src/cmd_github.rs Outdated Show resolved Hide resolved
crates/noseyparker/src/github/client.rs Outdated Show resolved Hide resolved
crates/noseyparker/src/github/models.rs Outdated Show resolved Hide resolved
@seqre
Copy link
Contributor Author

seqre commented Feb 16, 2024

Access to the GHES instance for tests is problematic, so I have no idea how to test that functionality. The only tests I could write are to confirm the successful deserialization of OrganizationShort and the CLI returning an error when scanning is requested and a custom GitHub API URL is not provided.

@bradlarsen
Copy link
Collaborator

@seqre I made a commit with some additional test cases, a couple small tweaks, and a re-sync with the main branch. I couldn't figure out how to add those commits to this PR directly myself (sometimes that is possible?? maybe to do with "allow edits from maintainers" when creating the PR?).

Instead, I created a new PR with your changes here plus that: #130. You are still credited as a co-author on that in a way that GitHub understands: 855087a

Thank you for the contribution!

@bradlarsen bradlarsen closed this Feb 23, 2024
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