Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Fix asynchronously added variants #160

Merged
merged 1 commit into from
Jan 8, 2018
Merged

Fix asynchronously added variants #160

merged 1 commit into from
Jan 8, 2018

Conversation

passsy
Copy link
Contributor

@passsy passsy commented Jan 5, 2018

all should be used because variants may not be available at that time when .forEach is call. all will also be called for each variant which will added later. This means here is a race condition on some machines not adding the required tasks.

see StefMa/AndroidArtifacts#9

From the documentation:
https://docs.gradle.org/current/javadoc/org/gradle/api/DomainObjectCollection.html#all(groovy.lang.Closure)
Executes the given action against all objects in this collection, and any objects subsequently added to this collection.

"subsequently added" variants where ignored until now.

fixes #96

`all` should be used because variants may not be available at that time when `.forEach` is call. `all` will also be called for each variant which will added later. This means here is a race condition on some machines not adding the required tasks.

see StefMa/AndroidArtifacts#9

From the documentation:
https://docs.gradle.org/current/javadoc/org/gradle/api/DomainObjectCollection.html#all(groovy.lang.Closure)
Executes the given action against all objects in this collection, and any objects subsequently added to this collection.

"subsequently added" variants where ignored until now.

fixes  #96
@hal9002
Copy link

hal9002 commented Jan 5, 2018

Can one of the admins review this PR?

@StefMa
Copy link
Contributor

StefMa commented Jan 8, 2018

ok to test

Copy link
Contributor

@StefMa StefMa left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@StefMa
Copy link
Contributor

StefMa commented Jan 8, 2018

Can a admin look why it's hanging?!

@ataulm
Copy link
Contributor

ataulm commented Jan 8, 2018

ok to test

@ataulm
Copy link
Contributor

ataulm commented Jan 8, 2018

yeah not really sure what's going on here

image

there's no queued builds on the CI 🤔

@ataulm ataulm merged commit c98020e into novoda:master Jan 8, 2018
@passsy passsy deleted the patch-1 branch January 9, 2018 11:09
@StefMa StefMa mentioned this pull request Jan 26, 2018
@mr-archano mr-archano mentioned this pull request Mar 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skipping upload for missing file pom-default
4 participants