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-32503] Update dependency of beanutils #81

Closed
wants to merge 4 commits into from

Conversation

KostyaSha
Copy link
Member

@KostyaSha KostyaSha commented Oct 11, 2016

@KostyaSha
Copy link
Member Author

No PR builds? 0_o

@KostyaSha
Copy link
Member Author

Commit that introduced that test c3004de

@KostyaSha
Copy link
Member Author

Any help is welcome ;)

@KostyaSha
Copy link
Member Author

This test also fails on 1.8.3 that used in Jenkins.

@KostyaSha
Copy link
Member Author

@rsandell maybe you can help? :)

* It is also possible to transform it to `withXXX()` to have clear vision.
*/
@DataBoundSetter
// public void setItems(List<String> items) {
Copy link
Member

Choose a reason for hiding this comment

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

dead code

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course it dead code, because it WIP

public FluentSetter() {}

/**
* WIP This tests passes in stapler, but fluent doesn't work in jenkins...
Copy link
Member

Choose a reason for hiding this comment

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

CC @vivek

KostyaSha referenced this pull request in jenkinsci/freestyle-multibranch-plugin Dec 14, 2016
@jglick
Copy link
Member

jglick commented Jan 18, 2017

DataBindingTest failing.

@KostyaSha
Copy link
Member Author

KostyaSha commented Jan 20, 2017

DataBindingTest failing.

it's known, i have no power to fix scalarTest that already in failure state for current jenkins core and seems i'm missing something...

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.

👍 excepting the dead code concern

@oleg-nenashev
Copy link
Member

@KostyaSha could you please rebase it to the master

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.

It could be shipped IMHO. though it's still marked as WiP. PTAL @jglick

@@ -44,7 +44,7 @@
<dependency>
<groupId>commons-beanutils</groupId>
<artifactId>commons-beanutils</artifactId>
<version>1.7.0</version>
<version>1.9.3</version>
Copy link
Member

Choose a reason for hiding this comment

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

Too new, and see #134.

Choose a reason for hiding this comment

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

Can this be revisited? We are running into some serious dependency issues because its at 1.8.3

*/
@DataBoundSetter
// public void setItems(List<String> items) {
public FluentSetter setItems(List<String> items) {
Copy link
Member

Choose a reason for hiding this comment

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

Seems potentially useful, but why is it filed here? Should be split into a separate PR.

Copy link
Member Author

@KostyaSha KostyaSha Oct 21, 2017

Choose a reason for hiding this comment

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

Fluentsetters was exactly the reason to update beanutils that provides FluentPropertyBeanIntrospector to be used in core... But i had working testcase here and failing in core + unclear test failure stuck me.

@KostyaSha
Copy link
Member Author

KostyaSha commented Oct 20, 2017 via email

@jglick
Copy link
Member

jglick commented Jun 11, 2020

Compare #188.

@jglick jglick marked this pull request as draft June 11, 2020 16:34
@jglick jglick changed the title WIP [JENKINS-32503] Update dependency of beanutils [JENKINS-32503] Update dependency of beanutils Jun 11, 2020
@KostyaSha
Copy link
Member Author

KostyaSha commented Sep 21, 2020

@jglick sp if everything is solved now, then core and test could simplify code with using fluent setters. Requires changes to return this instead void. Some tests on core side probably required. And PR could be closed.

@jglick
Copy link
Member

jglick commented Sep 23, 2020

Potentially useful new API, but would need various sorts of work to be mergeable.

@jglick jglick closed this Sep 23, 2020
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.

4 participants