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

Feature descriptor optional and check consistency #845

Closed
wants to merge 3 commits into
base: 0.13
from

Conversation

Projects
None yet
4 participants
@willb
Contributor

willb commented Aug 13, 2013

These commits refactor ConfigurationParser.getRepositories and add two additional repository options: descriptorOptional and skipConsistencyCheck. These correspond to the Ivy API calls r.setDescriptor(BasicResolver.DESCRIPTOR_OPTIONAL) and r.setCheckconsistency(false), where r is the resolver in question. The modifications pass all of the tests in the sbt test task and a couple of additional tests added to exercise the additional flexibility available in ConfigurationParser.

@harrah

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 14, 2013

Member

Thanks for the cleanup in ConfigurationParser, splitting the commits up appropriately, for expanding the tests, and handling launcher compatibility! Also, I appreciate you addressing both descriptorOptional and checkConsistency in this pull request. Further comments inline...

Member

harrah commented Aug 14, 2013

Thanks for the cleanup in ConfigurationParser, splitting the commits up appropriately, for expanding the tests, and handling launcher compatibility! Also, I appreciate you addressing both descriptorOptional and checkConsistency in this pull request. Further comments inline...

willb added some commits Aug 12, 2013

Additional options for Ivy resolvers.
Specify an Ivy resolver with ", descriptorOptional" to make Ivy
descriptor files optional for that repository or with
", skipConsistencyCheck" to disable Ivy consistency checks for
that repository.
Add factory methods to PatternsBasedRepository
This commit adds convenience methods to PatternsBasedRepository that
produce a copy of the repository with metadata optional or with
consistency checking disabled.
@harrah

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 26, 2013

Member

Sorry, I didn't intentionally ignore this, but unfortunately GitHub doesn't notify me when new commits are pushed. Is this ready to be merged?

Member

harrah commented Aug 26, 2013

Sorry, I didn't intentionally ignore this, but unfortunately GitHub doesn't notify me when new commits are pushed. Is this ready to be merged?

@willb

This comment has been minimized.

Show comment
Hide comment
@willb

willb Aug 26, 2013

Contributor

Yes, I made all of the changes from your comments. Thanks!

best,
wb

On Aug 26, 2013, at 9:47 AM, Mark Harrah notifications@github.com wrote:

Sorry, I didn't intentionally ignore this, but unfortunately GitHub doesn't notify me when new commits are pushed. Is this ready to be merged?


Reply to this email directly or view it on GitHub.

Contributor

willb commented Aug 26, 2013

Yes, I made all of the changes from your comments. Thanks!

best,
wb

On Aug 26, 2013, at 9:47 AM, Mark Harrah notifications@github.com wrote:

Sorry, I didn't intentionally ignore this, but unfortunately GitHub doesn't notify me when new commits are pushed. Is this ready to be merged?


Reply to this email directly or view it on GitHub.

@harrah

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 26, 2013

Member

Merged, thanks! Post-review by @eed3si9n or @jsuereth to verify binary/source compatibility.

Member

harrah commented Aug 26, 2013

Merged, thanks! Post-review by @eed3si9n or @jsuereth to verify binary/source compatibility.

@harrah harrah closed this Aug 26, 2013

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Aug 26, 2013

The original had two params with default: descriptorOptional: Boolean = false and skipConsistencyCheck: Boolean = false.
If someone had a code that passes an arg for descriptorOptional, but not skipConsistencyCheck, it won't compile, so this commit breaks the source compat.

eed3si9n commented on ivy/src/main/scala/sbt/Resolver.scala in fca8d99 Aug 26, 2013

The original had two params with default: descriptorOptional: Boolean = false and skipConsistencyCheck: Boolean = false.
If someone had a code that passes an arg for descriptorOptional, but not skipConsistencyCheck, it won't compile, so this commit breaks the source compat.

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 26, 2013

That got me too at first. It was introduced in a prior commit in this series: 7dafa48#L1L47.

harrah replied Aug 26, 2013

That got me too at first. It was introduced in a prior commit in this series: 7dafa48#L1L47.

@eed3si9n

This comment has been minimized.

Show comment
Hide comment
@eed3si9n

eed3si9n Aug 26, 2013

@willb @harrah Not sure how strict you want to be with binary compatibility, but wouldn't Ivy has to add older Ivy.apply and older constructor signature (like the Patterns above) in case some plugin is using it?

@willb @harrah Not sure how strict you want to be with binary compatibility, but wouldn't Ivy has to add older Ivy.apply and older constructor signature (like the Patterns above) in case some plugin is using it?

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 26, 2013

In the launcher, I think we should be fine only keeping binary compatibility via the launcher interfaces.

harrah replied Aug 26, 2013

In the launcher, I think we should be fine only keeping binary compatibility via the launcher interfaces.

@harrah

This comment has been minimized.

Show comment
Hide comment
@harrah

harrah Aug 26, 2013

Member

Thanks for the review Eugene.

Member

harrah commented Aug 26, 2013

Thanks for the review Eugene.

@willb

This comment has been minimized.

Show comment
Hide comment
@willb

willb Aug 26, 2013

Contributor

Thanks, Mark and Eugene!

Contributor

willb commented Aug 26, 2013

Thanks, Mark and Eugene!

@shijinkui

This comment has been minimized.

Show comment
Hide comment
@shijinkui

shijinkui Jul 27, 2018

hi, @willb how to set skipConsistencyCheck=ture. In the repositories file, I don't find how to set this parameter. If you know, please tell me. Thanks.

shijinkui commented Jul 27, 2018

hi, @willb how to set skipConsistencyCheck=ture. In the repositories file, I don't find how to set this parameter. If you know, please tell me. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment