Pix4D / cogito

Concourse resource for GitHub Commit Status and Google Chat notifications
MIT License
34 stars 14 forks source link

PCI-3363 add support for Github Enterprise API endpoint #143

Closed aliculPix4D closed 11 months ago

aliculPix4D commented 11 months ago

Based on: https://github.com/Pix4D/cogito/pull/139

The first commit is a squashed commit of all commits present in the above PR. Credits are given to both authors: @wanderanimrod and @dev-jin Thank you both for your work and first implementation.

The second commit simplifies a bit their work and it's basically a cleanup of the GhAPI field from top-level code. There was no need to keep using both GhAPI and GithubApiEndpoint.

While on it, I tried cleanup a bit the helper functions to prevent the unwanted bugs due to hardcoding github.com domain.

Closes: https://github.com/Pix4D/cogito/issues/51

TODO:

aliculPix4D commented 11 months ago

@wanderanimrod I think you can already pull the docker image from this branch: docker pull pix4d/cogito:pci-3363-support-ghe I would appreciate if you could test it a bit in your Concourse deployment and give us your feedback, since we don't have access to Github Enterprise.

dev-jin commented 11 months ago

Hi @aliculPix4D, I've successfully tested the pix4d/cogito:pci-3363-support-ghe image on our GHE instance and have confirmed that functionality is working as expected.

wanderanimrod commented 11 months ago

@aliculPix4D , @dev-jin has been working with me on this. The tests on our end are complete.

aliculPix4D commented 11 months ago

@marco-m-pix4d just for your info: sadly I was correct. There is an implicit relationship also with Gchat putter. https://github.com/Pix4D/cogito/blob/9558fa3fe5454cc1e1ff44c144cef9786862485b/cogito/gchatsink.go#L125 and the value is hardcoded there. This keeps getting better and better.

marco-m-pix4d commented 11 months ago

@aliculPix4D

sadly I was correct. There is an implicit relationship also with Gchat putter.

Good catch.

As mentioned face to face, we can split this in two. First we focus on delivering cogito for Enterprise GitHub. The release notes will mention in section "Known bugs" that the URL in the gchat message will be wrong.

Then, as a separate ticket, we make the gchat message correct also for Enterprise GitHub.

aliculPix4D commented 11 months ago

Closing in favor of: https://github.com/Pix4D/cogito/pull/143

SpectreVert commented 11 months ago

Closing in favor of: #143

146 no?

aliculPix4D commented 11 months ago

Closing in favor of: #143

146 no?

Yes, it was wrong, but more precisely: https://github.com/Pix4D/cogito/pull/144 and https://github.com/Pix4D/cogito/pull/145