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

GitLab MR formatter #351

Merged
merged 5 commits into from
Sep 19, 2019
Merged

Conversation

dsander
Copy link
Contributor

@dsander dsander commented Jul 14, 2019

This is based on the work of @Strnadj in #308.

Since he started GitLab added pipelines for merge requests which removes the need to guess the merge request for the current branch/commit. In addition to that it also re-runs jobs when the merge request target branch is changed.

Jan Strnadek and others added 5 commits July 13, 2019 12:04
Co-Authored-By: Willian van der Velde <mail@willian.io>
Since, as the name implies, the merge request formatter comments on
merge requests we can require that the GitLab CI jobs run in a
[merge request pipeline][1] which automatically sets
[``CI_MERGE_REQUEST_IID``][2]. This avoids having to guess the
merge request id based on the branch/commit which could lead to false
positives.

[1]: https://docs.gitlab.com/ee/ci/merge_request_pipelines/
[2]: https://docs.gitlab.com/ce/ci/variables/predefined_variables.html
@dsander dsander mentioned this pull request Jul 14, 2019
2 tasks
@carlallen
Copy link

I don't like this choice as it requires gitlab premium. It's nice that the version in my pull request #349 functions out of the box with gitlab core/starter

@dsander
Copy link
Contributor Author

dsander commented Jul 22, 2019

It doesn't require any premium feature, we are running this branch on the pure open source CE of GitLab.

I have not seen your PR before, if I did I wouldn't have opened this one :/

I removed the branch guessing because I thought a smaller change would be easier for the maintainers, but I am obviously also happy with our changes (even though adding CI_MERGE_REQUEST_IID in env_pull_id would be nice for not having to configure anything).

@carlallen
Copy link

I could have sworn pipleines for merge requests was a preimum feature, guess I was wrong...

@fneves
Copy link

fneves commented Aug 20, 2019

Hey all. Any news about the state of this PR?

@carlallen
Copy link

I can close #349, then I would assume this is ready for merge

@Sarin1204
Copy link

Just tested this and it works pretty well. Can the maintainers merge this and release a new gem version?

@carlallen
Copy link

Any updates on when this might get merged?

@doomspork doomspork merged commit c2b8d4c into prontolabs:master Sep 19, 2019
@doomspork
Copy link
Member

Sorry for the delay all. I'm the primary maintainer these days it seems and I'm currently recovering from a pretty serious back injury that is limiting my time.

apiology pushed a commit to apiology/pronto that referenced this pull request Dec 27, 2019
* Gitlab formatter

* Update README.md

Co-Authored-By: Willian van der Velde <mail@willian.io>

* Fix the specs

* Require `gitlab_mr` formatter to be run in a merge request pipeline

Since, as the name implies, the merge request formatter comments on
merge requests we can require that the GitLab CI jobs run in a
[merge request pipeline][1] which automatically sets
[``CI_MERGE_REQUEST_IID``][2]. This avoids having to guess the
merge request id based on the branch/commit which could lead to false
positives.

[1]: https://docs.gitlab.com/ee/ci/merge_request_pipelines/
[2]: https://docs.gitlab.com/ce/ci/variables/predefined_variables.html

* Update README
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.

6 participants