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-41591] Fix pipeline support using TransientActionFactory #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Dohbedoh
Copy link

@reviewbybees

Fix pipeline support, using a TransientActionFactory<Job> and a TransientActionFactory<Run>.

Note: Existing builds may have a duplicate "Build" link in the UI because of the non transient action added by previous version of the plugin. We could add a "migration" method although this will be fixed over time once new builds are being scheduled and old build discarded...

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Some breaking changes, but it should work well. Some integration tests would not hurt

@jcwilson
Copy link

I'm a user of this plugin (thanks, it's been really helpful!) and would love to see this update landed. Sorry I can't add more than my verbal support here.

@Dohbedoh Dohbedoh force-pushed the bugfix/JENKINS-41591 branch from 828fd8c to 2c0da93 Compare January 17, 2020 02:15
@Dohbedoh
Copy link
Author

Rebased to master.

@GLundh
Copy link
Member

GLundh commented Jan 17, 2020

I'm fine with this. I'll take en extra look this evening, and we'll see if can get it merged.

@GLundh
Copy link
Member

GLundh commented Jan 18, 2020

Hmm.. Can you make the old action invisible (e.g. nulling the url/name/image) and allow the transient action factory to produce a superclass of the RebuildAction (TransientRebuildAction or similar) and allow that one to be visible instead? This way you could ensure only one action is visible, but don't have to write the boring migration stuff (or resave all the builds).

@GLundh
Copy link
Member

GLundh commented Feb 4, 2020

Ping @Dohbedoh

@GLundh GLundh force-pushed the master branch 2 times, most recently from 370af40 to 316c049 Compare March 3, 2021 22:50
@Dohbedoh
Copy link
Author

@GLundh Added a transient field to control this.

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.

5 participants