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

Migrate binding priority to int-based system #934

Merged
merged 9 commits into from
Apr 3, 2024
Merged

Conversation

ZacSweers
Copy link
Collaborator

This allows more granular prioritization while also being ABI-compatible. See addition to the CHANGELOG for migration steps.

This allows more granular prioritization while also being ABI-compatible. See addition to the CHANGELOG for migration steps.
@ZacSweers ZacSweers requested a review from RBusarow March 15, 2024 18:50
@ZacSweers
Copy link
Collaborator Author

I'm very confused by these test failures, ContributesBindingGeneratorTest all pass locally for me 🤔

RBusarow added a commit that referenced this pull request Mar 29, 2024
prompted by #934 (comment)

Before this change, `allWarningsAsErrors` defaulted to `false` locally but `true` in CI.  We don't really gain anything by ignoring those warnings locally, since the relevant code is so isolated.

Note that the public `compileAnvil(...)` function in our test fixtures ([link](https://github.com/square/anvil/blob/a1def6af59ec11b9d92426947f8c833fb27e7bde/compiler-utils/src/testFixtures/java/com/squareup/anvil/compiler/internal/testing/AnvilCompilation.kt#L251)) defaults to `false`, but now our internal `compile(...)` from `TestUtils` now defaults to `true`.  That avoids a breaking change.
RBusarow added a commit that referenced this pull request Mar 29, 2024
prompted by #934 (comment)

Before this change, `allWarningsAsErrors` defaulted to `false` locally but `true` in CI.  We don't really gain anything by ignoring those warnings locally, since the relevant code is so isolated.

Note that the public `compileAnvil(...)` function in our test fixtures ([link](https://github.com/square/anvil/blob/a1def6af59ec11b9d92426947f8c833fb27e7bde/compiler-utils/src/testFixtures/java/com/squareup/anvil/compiler/internal/testing/AnvilCompilation.kt#L251)) defaults to `false`, but now our internal `compile(...)` from `TestUtils` now defaults to `true`.  That avoids a breaking change.
…tributesBindingGeneratorTest.kt

Co-authored-by: Rick Busarow <[email protected]>
@ZacSweers
Copy link
Collaborator Author

Oh oops I committed before I saw you'd pushed new commits

@RBusarow
Copy link
Collaborator

😕 I did? Didn't mean to!

RBusarow added a commit that referenced this pull request Mar 29, 2024
prompted by #934 (comment)

Before this change, `allWarningsAsErrors` defaulted to `false` locally but `true` in CI.  We don't really gain anything by ignoring those warnings locally, since the relevant code is so isolated.

Note that the public `compileAnvil(...)` function in our test fixtures ([link](https://github.com/square/anvil/blob/a1def6af59ec11b9d92426947f8c833fb27e7bde/compiler-utils/src/testFixtures/java/com/squareup/anvil/compiler/internal/testing/AnvilCompilation.kt#L251)) defaults to `false`, but now our internal `compile(...)` from `TestUtils` now defaults to `true`.  That avoids a breaking change.
@ZacSweers ZacSweers enabled auto-merge (squash) April 3, 2024 17:18
@ZacSweers ZacSweers merged commit 8ee5871 into main Apr 3, 2024
17 checks passed
@ZacSweers ZacSweers deleted the z/priorityInt branch April 3, 2024 17:55
RBusarow added a commit that referenced this pull request Apr 6, 2024
prompted by #934 (comment)

Before this change, `allWarningsAsErrors` defaulted to `false` locally but `true` in CI.  We don't really gain anything by ignoring those warnings locally, since the relevant code is so isolated.

Note that the public `compileAnvil(...)` function in our test fixtures ([link](https://github.com/square/anvil/blob/a1def6af59ec11b9d92426947f8c833fb27e7bde/compiler-utils/src/testFixtures/java/com/squareup/anvil/compiler/internal/testing/AnvilCompilation.kt#L251)) defaults to `false`, but now our internal `compile(...)` from `TestUtils` now defaults to `true`.  That avoids a breaking change.
RBusarow added a commit that referenced this pull request Apr 9, 2024
The published `AnvilCompilation` logic gets a new
`expectExitCode: ExitCode? = null` parameter.
If a value is specified,
the assertion is done before executing the `block` lambda.

In our internal delegating `compile(...)`
functions, the parameter becomes non-nullable and defaults to `ExitCode.OK`.

This is done in follow-up to
[this](#934 (comment))
comment.
RBusarow added a commit that referenced this pull request Apr 9, 2024
RBusarow added a commit that referenced this pull request Apr 9, 2024
prompted by #934 (comment)

Before this change, `allWarningsAsErrors` defaulted to `false` locally but `true` in CI.  We don't really gain anything by ignoring those warnings locally, since the relevant code is so isolated.

Note that the public `compileAnvil(...)` function in our test fixtures ([link](https://github.com/square/anvil/blob/a1def6af59ec11b9d92426947f8c833fb27e7bde/compiler-utils/src/testFixtures/java/com/squareup/anvil/compiler/internal/testing/AnvilCompilation.kt#L251)) defaults to `false`, but now our internal `compile(...)` from `TestUtils` now defaults to `true`.  That avoids a breaking change.
RBusarow added a commit that referenced this pull request Apr 9, 2024
* Migrate binding priority to int-based system

This allows more granular prioritization while also being ABI-compatible. See addition to the CHANGELOG for migration steps.

* Dodge -Werror

* More dodging

* Map name accordingly

* Or not

* Ok got it

* Update compiler/src/test/java/com/squareup/anvil/compiler/codegen/ContributesBindingGeneratorTest.kt

Co-authored-by: Rick Busarow <[email protected]>

---------

Co-authored-by: Rick Busarow <[email protected]>
RBusarow added a commit that referenced this pull request Apr 16, 2024
The published `AnvilCompilation` logic gets a new
`expectExitCode: ExitCode? = null` parameter.
If a value is specified,
the assertion is done before executing the `block` lambda.

In our internal delegating `compile(...)`
functions, the parameter becomes non-nullable and defaults to `ExitCode.OK`.

This is done in follow-up to
[this](#934 (comment))
comment.
RBusarow added a commit that referenced this pull request Apr 16, 2024
The published `AnvilCompilation` logic gets a new
`expectExitCode: ExitCode? = null` parameter.
If a value is specified,
the assertion is done before executing the `block` lambda.

In our internal delegating `compile(...)`
functions, the parameter becomes non-nullable and defaults to `ExitCode.OK`.

This is done in follow-up to
[this](#934 (comment))
comment.
RBusarow added a commit that referenced this pull request Apr 16, 2024
The published `AnvilCompilation` logic gets a new
`expectExitCode: ExitCode? = null` parameter.
If a value is specified,
the assertion is done before executing the `block` lambda.

In our internal delegating `compile(...)`
functions, the parameter becomes non-nullable and defaults to `ExitCode.OK`.

This is done in follow-up to
[this](#934 (comment))
comment.
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