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

Add GitLabCI as a supported platform #98

Closed
kitos9112 opened this issue Jun 6, 2021 · 13 comments · Fixed by #517
Closed

Add GitLabCI as a supported platform #98

kitos9112 opened this issue Jun 6, 2021 · 13 comments · Fixed by #517
Labels
enhancement New feature or request

Comments

@kitos9112
Copy link
Contributor

We've been using your forked version of tfnotify for quite a long time now flawlessly 🥳 got to admit, as it tackled a couple of notorious issues left behind by the original mercari creators. Nonetheless, after discovering your contributions to this repository, we also really want to give it a go!

As per the title of this GH issue, we would love to see support added to another well-known CI system, GitLab. We understand you may be extremely busy so I believe I could be of some help if you agree on reviewing my contributions

@suzuki-shunsuke suzuki-shunsuke added the enhancement New feature or request label Jun 6, 2021
@suzuki-shunsuke
Copy link
Owner

Thank you for your proposal.

GitLab is popular so this proposal is reasonable.

But I don't use GitLab, so it is hard to support GitLab.
By removing feature which I don't use from tfnotify, I make tfcmt simple and improve the maintainability.
I want to avoid that tfcmt becomes complicated by supporting GitLab.

But I think it is great if tfcmt supports GitLab.
I always welcome to your contribution.

@kitos9112
Copy link
Contributor Author

Right - This will need to be accomplished on a best-effort approach. We could back port some of the code developed by the chaps at mercari, however, by the look of it, your GitHub implementation goes beyond the initial tfnotify implementation with some further function decorators in the API interface for the notifier. I reckon we can work on a very simple prototype that focuses on adding comments to GitLab Merge Requests (method --> RepositoriesCreateComment)

@kitos9112
Copy link
Contributor Author

Hello @suzuki-shunsuke

Apologies but I've been dragged into so many commitments in the last 6 months that I couldn't pay particular attention to this enhancement I had proposed. Nevertheless, it seems I might manage to start coding on this shortly, I just need to get my head around a couple of other repositories that are consumed in here.

A few questions came to my mind after digging a little bit into it.

1- Would you see relevant adding gitlab as a supported CI env Platform interface in your own repository to abstract a few internal API calls within this repository?
2- Do you agree to tackle GitLab as a PoC/prototype for a couple of pre-releases until we are happy we the result? I understand you wanted to simplify all API operations and remove all other notifiers, but I reckon that adding very basic functionality would attract other peers using GitLab for their IaC integrations.

Also, I've contributed to the GitLab Terraform provider and I believe we could run integration/acceptance tests using their setup scripts in order to spin up a docker GitLab CE instance to run the tests againsts.

@kitos9112
Copy link
Contributor Author

@suzuki-shunsuke Any thoughts about my thoughts outlined above?

@hirosassa
Copy link
Contributor

@suzuki-shunsuke @kitos9112 I'd like to create PR for this. Is it OK?

@suzuki-shunsuke
Copy link
Owner

Sorry for late reply.

Unfortunately, I don't have motivation to support GitLab now.
I don't use GitLab, so it's difficult to deal with feature request and bug report about GitLab continuously.
And I'd like to keep tfcmt's implementation simple as much as possible.

@hirosassa
Copy link
Contributor

@suzuki-shunsuke Thank you for your reply! I understand your opinion. I'll fork this repo and develop the feature for supporting GitLab.

@kitos9112
Copy link
Contributor Author

@hirosassa absolutely! I couldn't spare more than a couple of hours several months ago. Had a local draft in which I had tried to follow @suzuki-shunsuke logic.
Please, share the fork details whenever you begin the work!

@hirosassa
Copy link
Contributor

hirosassa commented Apr 30, 2022

@suzuki-shunsuke @kitos9112
FYI: I released v0.1.1 of tfcmt-gitlab 🎉 . This is still rough implementation but it worked in my private gitlab environment.
Please try!
https://github.com/hirosassa/tfcmt-gitlab/releases/tag/v0.1.1

Thanks,

@suzuki-shunsuke
Copy link
Owner

@kitos9112
Copy link
Contributor Author

@hirosassa - Looks cool! Will give it a go next week.

@kitos9112
Copy link
Contributor Author

@hirosassa - First impressions are good. I was a little bit sceptical in the beginning as there's not much documentation on default ENV variables, but after sneaking into some of the code I realised you aligned with the GitlabCI naming convention.

So happy days. I'll start rolling it out in our dev CI flows!

@hirosassa
Copy link
Contributor

@kitos9112 Thanks for your feedback.

there's not much documentation on default ENV variables

Yeah, I have to prepare it. The PR for this is also welcome 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants