Fix bug of sending the status in case of a merge request from fork#39
Fix bug of sending the status in case of a merge request from fork#39LukinovD wants to merge 1 commit intoJetBrains:Indore-2017.2.xfrom
Conversation
|
bump! |
|
Hi @Frodox, we are currently reviewing the procedure of accepting pull requests into open source TeamCity plugins from legal perspective. Otherwise the fix seems to be useful and we are planning to accept it. |
|
@LukinovD did I get it right that the credentials used by Commit Status Publisher should allow publishing statuses to the source project too? |
|
@karanagai, yes, it is true |
| MRInfoResponseProcessor processorMR = new MRInfoResponseProcessor(); | ||
|
|
||
| try { | ||
| HttpHelper.get(url, null, null, Collections.singletonMap("PRIVATE-TOKEN", getPrivateToken()), BaseCommitStatusPublisher.DEFAULT_CONNECTION_TIMEOUT, processorMR); |
There was a problem hiding this comment.
The HTTP request is made synchronously within the CommitStatusPublisherListener event handlers execution. It may cause the control not to be returned to the code thatr calls these event handlers promptly enough, which is not desirable.
| String message = createMessage(status, build, revision, myLinks.getViewResultsUrl(build), description); | ||
|
|
||
| Pattern r = Pattern.compile("/merge-requests/([0-9]+)/"); | ||
| Matcher m = r.matcher(revision.getRepositoryVersion().getVcsBranch()); |
There was a problem hiding this comment.
The return value of getVcsBranch() is marked as @Nullable
|
|
||
| Pattern r = Pattern.compile("/merge-requests/([0-9]+)/"); | ||
| Matcher m = r.matcher(revision.getRepositoryVersion().getVcsBranch()); | ||
| if (m.find( )) { |
There was a problem hiding this comment.
This is minor, but worth noticing: GitLab branch name may contain slashes, so it seems using find() you in theory may match a branch name that is not a merge request, but its name just contain something that is matched by the pattern.
|
@karanagai Is there a chance that you can merge this PR and fix those problems on your side? This is an annoying problem as more and more people use a self-hosted GitLab installation. |
|
Is there any estimation or a rejection of this feature? This is kind of game-breaker for us with TeamCity and it seemed it would work out of the box. Unfortunately the most used fork-pull-merge can't be used with it. |
5 participants
Fix bug of sending the status in case of a merge request from fork repository