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

Skip to next bundle on predicate failure. #78

Merged
merged 1 commit into from
Mar 25, 2016

Conversation

geomacy
Copy link
Contributor

@geomacy geomacy commented Mar 23, 2016

Without this change, if the predicate fails, the code flow just picks the next predicate to check,
when really we want to skip to checking the next bundle.

if (symbolicName!=null && !symbolicName.equals(b.getSymbolicName())) continue;
if (version!=null && !Version.parseVersion(version).equals(b.getVersion())) continue;
for (Predicate<? super Bundle> predicate: predicates) {
if (!predicate.apply(b)) continue;
if (!predicate.apply(b)) continue allBundles;
Copy link
Member

Choose a reason for hiding this comment

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

Could Predicates.and be used instead of the nested loop? Alternatively can you extract it in a separate method to avoid dealing with labels.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 i don't like labels here, and in any case i think the inner loop as svet suggests can be replaced by one line

if (!Predicates.and(predicates).apply(b)) continue;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds like a fix to me! will add that soon.

Without this change, if the predicate fails, the code flow just picks the next predicate to check,
when really we want to skip to checking the next bundle.
@geomacy geomacy changed the title Add a label to predicates check in findAll. Skip to next bundle on predicate failure. Mar 24, 2016
@geomacy
Copy link
Contributor Author

geomacy commented Mar 24, 2016

Have rebased and pushed the change suggested.

@neykov
Copy link
Member

neykov commented Mar 24, 2016

Kicked a new build due to the unrelated failure.

Results :

Tests run: 0, Failures: 0, Errors: 0, Skipped: 0

[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: Invalid object ID 34 iota=49
[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: Invalid object ID 34 iota=49
[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: Invalid object ID 27 iota=49

@geomacy
Copy link
Contributor Author

geomacy commented Mar 24, 2016

Did this not complete? The build seems to have finished successfully.

@geomacy
Copy link
Contributor Author

geomacy commented Mar 24, 2016

Trying a recycle

@geomacy geomacy closed this Mar 24, 2016
@geomacy
Copy link
Contributor Author

geomacy commented Mar 24, 2016

Reopening

@geomacy geomacy reopened this Mar 24, 2016
@asfgit asfgit merged commit 71932b2 into apache:master Mar 25, 2016
asfgit pushed a commit that referenced this pull request Mar 25, 2016
Skip to next bundle on predicate failure.

Without this change, if the predicate fails, the code flow just picks the next predicate to check,
when really we want to skip to checking the next bundle.
@geomacy geomacy deleted the osgis-loop-fix branch April 5, 2016 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants