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

Use standard Jenkins POM in Winstone, Refactor the project structure #46

Merged
merged 7 commits into from
Feb 28, 2018

Conversation

oleg-nenashev
Copy link
Member

During the release of 4.1.1 I hit the issue with source uploads, which was failing the release due to the duplication of artifacts. So I decided to fix it and to align pom.xml with other Jenkins components.

This change introduces Jenkins parent POM 1.37 in the project and restructures it to be compatible with default project layout in the organization.

@reviewbybees @olamy

This change introduces Jenkins parent POM 1.37 in the project and restructures it to be compatible with default project layout in the organization.
Copy link
Member

@dwnusbaum dwnusbaum left a comment

Choose a reason for hiding this comment

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

🐝

@@ -1,6 +1,8 @@
package winstone;

import junit.framework.TestCase;
import winstone.BoundedExecutorService;
import winstone.Launcher;
Copy link
Member

Choose a reason for hiding this comment

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

The packages haven't changed, right, so why are these necessary? I guess Launcher could be hudson.Launcher, but I'm not sure about BoundedExecutorService.

Copy link
Member Author

Choose a reason for hiding this comment

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

Likely it was a refactoring mess, will fix

@@ -27,6 +28,7 @@
* @author <a href="mailto:[email protected]">Rick Knowles</a>
* @version $Id: LoadTest.java,v 1.2 2006/02/28 07:32:49 rickknowles Exp $
*/
@Ignore
Copy link
Member

Choose a reason for hiding this comment

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

Maybe specify the reason: @Ignore("Intended to be run manually"). (At least I think that is why it was excluded previously.)

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed

Copy link
Member

@olamy olamy left a comment

Choose a reason for hiding this comment

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

LGTM I was really tempted as well :-)

@oleg-nenashev
Copy link
Member Author

Since it's not urgent, I will try to integrate a new parent POM with FindBugs support.
I am tired of updating every core component to get it :)

@olamy
Copy link
Member

olamy commented Feb 28, 2018

Hi @oleg-nenashev
I'd like to do some changes. So to avoid you or me some pain in merging maybe you should merge this one first?

@oleg-nenashev
Copy link
Member Author

oleg-nenashev commented Feb 28, 2018

@olamy will do within few hours

@oleg-nenashev
Copy link
Member Author

comments have been addressed

@@ -1,5 +1,5 @@
LoadTest.Usage=Winstone Command Line Load Tester\n\
Usage: java winstone.testCase.load.LoadTest --url=<url> \
Copy link
Member Author

Choose a reason for hiding this comment

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

refactoring mess

@oleg-nenashev oleg-nenashev changed the title Use standard Jenkins POM in Winstone. Use standard Jenkins POM in Winstone, Refactor the project structure Feb 28, 2018
@oleg-nenashev oleg-nenashev merged commit 9132cc3 into jenkinsci:master Feb 28, 2018
@oleg-nenashev
Copy link
Member Author

@olamy done, I hope it unblocks you

@oleg-nenashev
Copy link
Member Author

@olamy should I cut the release or wait for your patch to arrive?

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