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

Added PreBuildMerge as a Git Trait #513

Conversation

@arthurvanduynhoven
Copy link

arthurvanduynhoven commented Jul 19, 2017

For a Multibranch Pipeline job, we need to be able to pre-merge the Jenkinsfile so that the latest changes can be tested properly. This feature was available before the introductions to Git Traits.

@stephenc

This comment has been minimized.

Copy link
Member

stephenc commented Jul 20, 2017

I think this would be better in an extension plugin rather than a core plugin exposed to everyone. The git plugin is already complex enough, WDYT @MarkEWaite

Copy link
Member

stephenc left a comment

Actual change looks correct, but I would prefer this in an extension plugin rather than core git plugin behaviour

/*
* The MIT License
*
* Copyright (c) 2017 CloudBees, Inc.

This comment has been minimized.

Copy link
@stephenc

stephenc Jul 20, 2017

Member

@arthurvanduynhoven does not work for CloudBees last I checked

This comment has been minimized.

Copy link
@arthurvanduynhoven

arthurvanduynhoven Jul 21, 2017

Author

Sorry -- copy paste issue. I remove the mention of CloudBees.

@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Jul 21, 2017

@stephenc If we move this to an extension plugin, how would you want to handle the deprecation of PreBuildMerge class? As I would assume this logic should be migrated to the new extension plugin.

@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Aug 2, 2017

@stephenc

This comment has been minimized.

Copy link
Member

stephenc commented Aug 2, 2017

I don't think you need to steal the extension, you should be fine with just the trait

@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Aug 3, 2017

@stephenc The reason I "stole" the extension was to include the required methods
equals() hashCode() toString() and getDisplayName()
These changes are not required in the core git-plugin as from what I understand the changes are only really needed for the trait and in the end you want to remove PreBuildMerge from the core git-plugin?

But if you think I should open a new MR for these changes in the core git-plugin I can and then I will make the appropriate changes in the trait code.

@stephenc

This comment has been minimized.

Copy link
Member

stephenc commented Aug 3, 2017

I think the changes to the extension (which is currently in the git plugin) should live in the git plugin.

A second thing, Your extension plugin should ensure that your extension is limited to GitSCMSource and doesn't show for GitHub or the others as it will conflict with PR merges

@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Aug 3, 2017

(for Reference) I have added MR #517 for the core Git Changes.

For the second item to restrict only to GitSCMSource, doesn't GitSCMExtensionTrait make it only for GitSCMSource? Or do I need to change that to SCMSourceTrait and change DescriptorImpl to the following:

/**
     * Our {@link hudson.model.Descriptor}.
     */
    @Extension
    @Discovery
    public static class DescriptorImpl extends SCMSourceTraitDescriptor {

        /**
         * {@inheritDoc}
         */
        @Override
        public String getDisplayName() {
            return "Merge before build";
        }

        /**
         * {@inheritDoc}
         */
        @Override
        public Class<? extends SCMBuilder> getBuilderClass() {
            return GitSCMBuilder.class;
        }

        /**
         * {@inheritDoc}
         */
        @Override
        public Class<? extends SCMSourceContext> getContextClass() {
            return GitSCMSourceContext.class;
        }

        /**
         * {@inheritDoc}
         */
        @Override
        public Class<? extends SCMSource> getSourceClass() {
            return GitSCMSource.class;
        }
    }
@stephenc

This comment has been minimized.

Copy link
Member

stephenc commented Aug 3, 2017

You can just override the methods in your trait's descriptor

@stephenc

This comment has been minimized.

Copy link
Member

stephenc commented Aug 3, 2017

IOW in your GitSCMExtentionDescriptor impl just add

/**
         * {@inheritDoc}
         */
        @Override
        public Class<? extends SCMSource> getSourceClass() {
            return GitSCMSource.class;
        }
@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Aug 3, 2017

@stephenc Thanks for your help. I will push the latest changes to the new repo.
Do you know when Git-Plugin (3.5.1?) will be released next? As I will have a dependency on this newly release Git-Plugin due to the PreBuildMerge class changes.

@arthurvanduynhoven

This comment has been minimized.

Copy link
Author

arthurvanduynhoven commented Aug 3, 2017

Closing as not needed anymore.

For further tracking please refer to the following items:
Git-Plugin MR #517
Plugin Hosting Request: https://issues.jenkins-ci.org/browse/HOSTING-380

@MarkEWaite

This comment has been minimized.

Copy link

MarkEWaite commented Aug 8, 2017

Git plugin 3.5.1 released 5 Aug 2017.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.