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

[JENKINS-52914] - Allow decorating Launchers for runs, introduce a createLauncher API on the Run level #3577

Closed

Conversation

oleg-nenashev
Copy link
Member

Just detaching some code from #3575 which we will unlikely need in the current implementation. It may be still useful as overall generalization, please let me know if somebody finds it useful.

See JENKINS-52914.

Proposed changelog entries

  • RFE: Developer: Introduce Run#createLauncher() and LauncherDecorator#decorateFor(Run) API
  • ...

Submitter checklist

  • JIRA issue is well described
  • Changelog entry appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    * Use the Internal: prefix if the change has no user-visible impact (API, test frameworks, etc.)
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot see anything wrong with it, but we should not merge a new API with no known use case.

@@ -518,6 +555,8 @@ public Result run(@Nonnull BuildListener listener) throws Exception {
return result;
}



Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

@oleg-nenashev oleg-nenashev added the needs-justification This PR lacks a motivation for the proposed change. label Aug 10, 2018
@oleg-nenashev
Copy link
Member Author

Yes. Likely it needs a reference impl, e.g. in Pipeline or other plugin which creates lanchers

@batmat batmat added the unresolved-merge-conflict There is a merge conflict with the target branch. label Apr 4, 2019
@daniel-beck
Copy link
Member

This seems abandoned given the lack of progress and comments since August. Therefore closing as stalled.

@daniel-beck daniel-beck closed this Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-justification This PR lacks a motivation for the proposed change. unresolved-merge-conflict There is a merge conflict with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants