Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski What was the issue number for this fix?
    @proski Is there a lifecycle diagram for this plugin somewhere? :)
    Pavel Roskin
    @proski
    @gpaciga As I said, the fix for running checks on Stash errors didn't make it to 1.10. I hope to implement better logging eventually that would record the reason for building the PR, including the comment number if any.
    @wreimers_twitter That's issue #93. I don't think there is a lifecycle diagram for the plugin.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski Thanks.
    Gregory Paciga
    @gpaciga
    at least I have an explanation now as to why it's probably happening, thanks :)
    jakub-bochenski
    @jakub-bochenski
    New version 1.11 is out
    jakub-bochenski
    @jakub-bochenski
    @proski I merged a few PRs, can you try and resolve the ones that conflict now?
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @jakub-bochenski Thanks! Does this release include the fix for #93?
    Pavel Roskin
    @proski
    @wreimers_twitter Yes, the fix for #93 is included, and so is the pipeline support (#69).
    Pavel Roskin
    @proski
    @jakub-bochenski Thank you for merging my PRs and making a release! My remaining PRs have been rebased.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski Great, thanks!
    Pavel Roskin
    @proski
    I've updated the merge-all branch. It's much easier to merge the remaining PRs now with the Jackson2 migration behind us! It used SpotBugs with Low threshold and doesn't suppress any warnings. Note that SpotBugs still finds over 100 issues when used in Eclipse. Even with the Low threshold, SpotBugs doesn't report them when run non-interactively. I'm not trying to write perfect code, I'm just trying to make tools help us write better code.
    Pavel Roskin
    @proski
    These are the remaining issues with the plugin as I see them. HTTP error handling is still a mess. Reading all PR comments is unnecessary. The comments should be read until the decision is made no to build the PR or until all parameters have been filled. We need detailed logging why PRs are built or skipped (poll logging is good for skipped PRs; decisions to build a PR should be written to the system log as well). Comments should be edited, not deleted, as deletion kills responses to the comments. When the build starts, the comment should be edited to provide the build link. Custom messages should be configured in the trigger setup, not in the post-build action; that way they would be accessible to pipelines and easy for users to find.
    jakub-bochenski
    @jakub-bochenski

    Comments should be edited, not deleted, as deletion kills responses to the comments.

    I would really prefer to switch to a stash-notifier like approach using the build status instead. If you have multiple jobs the comments are really a nuisance and spam the notifications

    jakub-bochenski
    @jakub-bochenski
    Actually I just pushed out yet another release, mostly for the plugin-size reducing changes
    Pavel Roskin
    @proski
    @jakub-bochenski Thank you for merging almost all my PRs, it's great! I can start working on new big things. I'll check what we can do about the build status.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    Good morning. Does anyone have a Groovy snippet I can use to trigger a PR build using the plugin? I can't seem to trigger it from Stash using the trigger phrase "please test this".
    Oops, I meant "test this please".
    Pavel Roskin
    @proski
    @wreimers_twitter The plugin decides whether to build a PR solely on the PR status (title, comments, merge/conflict state). You would need to implement parts of addFutureBuildTasks and startJob from StashRepository.java in Groovy to imitate the plugin behavior.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski Ok, thanks.
    Yaron Tal
    @yaron
    We've just upgraded our jenkins to 2.187 and stash pullrequest builder plugin to 1.12 from 1.9 and it looks like the plugin isn't working anymore
    Getting this error in the jenkins logs:
    stashpullrequestbuilder.stashpullrequestbuilder.StashBuildTrigger.run() failed for org.jenkinsci.plugins.workflow.job.WorkflowJob@aa23771[......]
    java.lang.NullPointerException
    at stashpullrequestbuilder.stashpullrequestbuilder.StashBuildTrigger.run(StashBuildTrigger.java:309)
    at hudson.triggers.Trigger.checkTriggers(Trigger.java:278)
    Any pointers ?
    Pavel Roskin
    @proski
    @yaron: The line numbers don't match version 1.12 or version 1.9 of the plugin. Line 309 in StashBuildTrigger.java is not in the run() method in either version. The line number for Trigger.java is correct for Jenkins 2.187. It looks like you are trying to use the plugin with a pipeline. Pipelines were not supported in version 1.9. It that a new job? Do you have any jobs that are not pipelines? Are they working? Is pipeline support enabled in the global configuration? Do you get that log message every time Stash is polled? Can you check the credentials for the job? One possible way to get NullPointerException in StashBuildTrigger.java is to use deleted or unavailable credentials.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski Are pipelines supported in 1.12?
    Pavel Roskin
    @proski
    @wreimers_twitter Yes, they are. But you need to enable them in the global Jenkins configuration, as described in README.
    Ghost
    @ghost~5d41dde6d73408ce4fc76d93
    @proski Thanks.
    I missed that part!
    Liam
    @wreimers
    Switched to my GitHub account.
    Pavel Roskin
    @proski
    @yaron: I believe I have a fix for your issue. I think you got the line number wrong, but there is indeed a way how StashBuildTrigger can get a NullPointerException in the run() method. The PR is #141, I would appreciate if you test it.
    Yaron Tal
    @yaron
    Looks like we where using a slightly modified and older version that had some functionality added to support approving/declining pull requests, so that's why the line numbers where off
    Pavel Roskin
    @proski
    Try using Bitbucket Server Notifier plugin (former Stash Notifier). It wouldn't decline pull requests, but it would mark them as failed if the build fails, which would prevent the merge. I'm not interested in debugging a modified version, but if you can reproduce the issue with unmodified plugin, please post the backtrace here.
    jakub-bochenski
    @jakub-bochenski
    Merged #141 and pushed out a new release

    Try using Bitbucket Server Notifier plugin (former Stash Notifier). It wouldn't decline pull requests, but it would mark them as failed if the build fails, which would prevent the merge.

    Agree, if you want to prevent merges the notifier plugin is the way to go

    Gregory Paciga
    @gpaciga

    speaking of the notifier plugin, I'm trying to build based on the merge ref, but then the notifier plugin uses what seems like the ref to the merge result instead of the ref to the source commit. I see there's a line in the docs about this but I don't understand what it means:

    you need to set ${sourceCommitHash} as Commit SHA-1 to record the build result against the source commit.

    the builder plugin already sets ${sourceCommitHash}... so what do I have to do to get the notifier to pick that up?

    ah nevermind, I see.... I set it in the advanced settings of the post-build task. of course as soon as I ask I find it
    Gregory Paciga
    @gpaciga
    I've now noticed a few cases where Bitbucket receives the "Build started" comment, but in Jenkins no build is actually started. In one afternoon I have three new PRs open (3937, 3938, 3939) but Jenkins only records build 81 for the first and 82 for the third. What might have happened?
    Gregory Paciga
    @gpaciga
    ah, I think it's because of Jenkins only allowing one build in the queue... three PRs at once will put the 1st in progress, the 2nd in the queue, and then the 3rd will kick the 2nd out of the queue because Jenkins's assumption is that it will be out of date.
    is there any way around that other than allowing more parallel builds?
    Gregory Paciga
    @gpaciga
    the build queue can be >1 if there's a parameter... could somehow it be possible to have builds parametrized by the PR number or something?
    Gregory Paciga
    @gpaciga

    ah:

    If the project has a parameter with the name of one of those environment variables, the value of the parameter is replaced with the value of that environment variable.

    is it fair to say that this should be standard practice, to add a parameter for pullRequestId? I wonder if this should be noted in the "Creating a Job" section of the README

    Pavel Roskin
    @proski
    @gpaciga You are correct that the "Build started" should be "Build queued" first. Then it could be updated to "Build started" with a link, then to "Build finished". It's quite annoying to have a running build and not to have a link to it in the comment.
    Pavel Roskin
    @proski
    For the second comment, this may not be the best place to ask. More information is needed. I also suggest that you separate observations from assumptions. Jenkins doesn't assume that somebody would be out of date, it polls the triggers, and the triggers make such decisions. Maybe you only wave two executors? The number of executors is configurable in Manage Jenkins -> Configure System. Confusingly, it's shown under "Maven Project Configuration" on my installation of Jenkins 2.189, but it's not for Maven.
    Do you have "Cancel outdated jobs" enabled? That could explain some of your observations. I suggest that you turn it off. It's actually buggy. If Jenkins is preparing for restart, the plugin would cancel the old job and fail to start the new one. Yet it would keep the "Build started" comment, preventing further builds.
    Pavel Roskin
    @proski
    I would prefer that we don't ask users to create parameterized projects to deal with the issues that the plugin can do on its own. What benefits would adding the pullRequestId provide?
    You are welcome to create a pull request for README.md. I don't think chances are high that it would be approved, as adding any complexity to the setup is undesirable. But the pull request could provide a better place for the discussion and could lead to improvements in the documentation.
    Gregory Paciga
    @gpaciga
    if the plugin can do it on its own, then it should... any idea what work would be required to make that possible? am I wrong that not having this on by default basically breaks what the plugin is supposed to do?
    Pavel Roskin
    @proski
    @gpaciga I'm afraid I lost the context here. Please open a ticket on https://issues.jenkins-ci.org/ and post it here. Make sure to describe what you want to do, what you do and what you observe. Add relevant information about your setup, such as Jenkins version, Stash Pull Request Builder plugin version, number of executors, type of the project (freestyle, matrix, pipeline etc) and the plugin settings.
    Pavel Roskin
    @proski
    All assumptions and suggestions should be separate from the issue description. If you propose any changes, please be clear if you have tested them and whether they had the desired effect.