Skip to content

Upgrade to bval 2.0.0#167

Merged
electrum merged 1 commit intoairlift:masterfrom
findepi:findepi/master/upgrade-to-bval-2-0-0-79bd30
Nov 14, 2018
Merged

Upgrade to bval 2.0.0#167
electrum merged 1 commit intoairlift:masterfrom
findepi:findepi/master/upgrade-to-bval-2-0-0-79bd30

Conversation

@findepi
Copy link
Contributor

@findepi findepi commented Nov 14, 2018

No description provided.

@findepi
Copy link
Contributor Author

findepi commented Nov 14, 2018

@electrum Validation 2.0 brings ability to validate List and Optional elements, for example List<@Pattern(...) String>.

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

I'm thinking we should actually remove BVal from Airbase and just have it in Airlift, since no one needs to use it directly. It's an implementation detail of configuration and ValidationAssertions in testing. Thoughts?

<dep.logback.version>1.2.3</dep.logback.version>
<dep.javax-inject.version>1</dep.javax-inject.version>
<dep.javax-validation.version>1.1.0.Final</dep.javax-validation.version>
<dep.javax-validation.version>2.0.1.Final</dep.javax-validation.version>
Copy link
Member

Choose a reason for hiding this comment

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

Add this to CHANGES

airbase/pom.xml Outdated
<!-- misc stuff -->

<!-- bval 1.1.1 depends on commons-lang3 3.3.2, which is incompatible with Java 9 -->
<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

We can delete this dependency now. Based on the comment, it was added to force a newer version (than bval used) that is compatible with Java 9.

airbase/pom.xml Outdated
<artifactId>bval-jsr</artifactId>
<version>1.1.1</version>
<version>${dep.bval.version}</version>
<exclusions>
Copy link
Member

Choose a reason for hiding this comment

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

None of these exclusions should be needed now. I added it to a new project and dependency:tree shows no dependencies. Although it's strange that commons-weaver-privilizer-api is declared as a dependency but Maven doesn't pick it up.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see it's not picked up because the parent POM declares it as provided scope in dependencyManagement. I didn't realize you could do that (and it seems strange).

@electrum
Copy link
Member

Thinking about this more, since it appears the API didn't change, the only requirement is updating in Airbase (or otherwise bumping the dependency version). We don't need to change Airlift at all. So maybe leaving it in Airbase is better?

@findepi
Copy link
Contributor Author

findepi commented Nov 14, 2018

There is additional bonus when this stays in airbase -- when i want to do validation outside of airlift configuration, i will pull the matching bval version easily. And my use-case just happens to be that.

@findepi findepi force-pushed the findepi/master/upgrade-to-bval-2-0-0-79bd30 branch from eda6053 to 4b9f3dd Compare November 14, 2018 17:13
@electrum electrum merged commit 7435e2c into airlift:master Nov 14, 2018
@findepi findepi deleted the findepi/master/upgrade-to-bval-2-0-0-79bd30 branch November 14, 2018 22:37
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.

2 participants