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

fix: generate a select statement for each condition #1173

Merged
merged 8 commits into from
Sep 14, 2024

Conversation

roya1v
Copy link
Contributor

@roya1v roya1v commented Jul 16, 2024

This is should fix #1111 . I had the same issue where the select in cops would have multiple matching conditions, which I think isn't something wrong. So in here I changed the code so it generates separate selects for each option.

I'm not completely happy with the implementation and I still have to fix the tests probably, but please tell me if you think I'm going in a completely wrong direction

@roya1v roya1v changed the title Possible Progress fix for #1111 Possible fix for #1111 Jul 16, 2024
@cgrindel
Copy link
Owner

Instead of creating a separate to_multiple_starlark() function, can we integrate this logic into the existing to_starlark()?

@roya1v
Copy link
Contributor Author

roya1v commented Sep 7, 2024

Hey @cgrindel ! Sorry for the long wait, I've combined the two functions like you suggested, let me know what you think

@cgrindel cgrindel changed the title Possible fix for #1111 fix: generate a select statement for each condition Sep 7, 2024
Copy link
Owner

@cgrindel cgrindel left a comment

Choose a reason for hiding this comment

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

Overall, looks good. It appears that some unit tests need to be updated.

@luispadron Does this solution solve the original issue that you filed?

swiftpkg/internal/bzl_selects.bzl Outdated Show resolved Hide resolved
@cgrindel cgrindel marked this pull request as ready for review September 7, 2024 17:57
Copy link
Collaborator

@luispadron luispadron left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, sorry it's taken so long to review!

I'm out of office this week but I can double check if this solves our issue soon, if I forget please remind me!

It would also be nice to either figure out how to test this edge case in a fixture or to also build our current examples in release mode to see if that captures this case already.

swiftpkg/internal/bzl_selects.bzl Outdated Show resolved Hide resolved
@cgrindel
Copy link
Owner

@Mergifyio queue

Copy link
Contributor

mergify bot commented Sep 14, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at f5364a9

@mergify mergify bot merged commit f5364a9 into cgrindel:main Sep 14, 2024
33 checks passed
renovate bot added a commit to bazel-contrib/rules_bazel_integration_test that referenced this pull request Sep 17, 2024
…#358)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
|
[rules_swift_package_manager](https://redirect.github.com/cgrindel/rules_swift_package_manager)
| http_archive | minor | `v0.37.0` -> `v0.38.2` |

---

### Release Notes

<details>
<summary>cgrindel/rules_swift_package_manager
(rules_swift_package_manager)</summary>

###
[`v0.38.2`](https://redirect.github.com/cgrindel/rules_swift_package_manager/releases/tag/v0.38.2)

[Compare
Source](https://redirect.github.com/cgrindel/rules_swift_package_manager/compare/v0.38.1...v0.38.2)

#### What Has Changed

#### What's Changed

- chore: update README.md for v0.38.1 by
[@&#8203;cgrindel-app-token-generator](https://redirect.github.com/cgrindel-app-token-generator)
in
[cgrindel/rules_swift_package_manager#1242

**Full Changelog**:
cgrindel/rules_swift_package_manager@v0.38.1...v0.38.2

#### Bazel Module Snippet

```python
bazel_dep(name = "rules_swift_package_manager", version = "0.38.2")
```

###
[`v0.38.1`](https://redirect.github.com/cgrindel/rules_swift_package_manager/releases/tag/v0.38.1)

[Compare
Source](https://redirect.github.com/cgrindel/rules_swift_package_manager/compare/v0.38.0...v0.38.1)

#### What Has Changed

#### What's Changed

- chore(deps): update dependency sdwebimage/sdwebimageswiftui to from:
"3.1.2" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1213

**Full Changelog**:
cgrindel/rules_swift_package_manager@v0.38.0...v0.38.1

#### Bazel Module Snippet

```python
bazel_dep(name = "rules_swift_package_manager", version = "0.38.1")
```

###
[`v0.38.0`](https://redirect.github.com/cgrindel/rules_swift_package_manager/releases/tag/v0.38.0)

[Compare
Source](https://redirect.github.com/cgrindel/rules_swift_package_manager/compare/v0.37.0...v0.38.0)

#### What Has Changed

#### What's Changed

- chore: update README.md for v0.37.0 by
[@&#8203;cgrindel-app-token-generator](https://redirect.github.com/cgrindel-app-token-generator)
in
[cgrindel/rules_swift_package_manager#1216
- chore(deps): update dependency gazelle to v0.38.0 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1193
- chore(deps): update dependency rules_apple to v3.8.0 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1196
- chore(deps): update dependency apple/swift-nio to v2.72.0 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1219
- fix: generate a `select` statement for each condition by
[@&#8203;roya1v](https://redirect.github.com/roya1v) in
[cgrindel/rules_swift_package_manager#1173
- fix: ignore unresolved packages in Package.swift by
[@&#8203;luispadron](https://redirect.github.com/luispadron) in
[cgrindel/rules_swift_package_manager#1223
- chore(deps): update dependency googlemaps/ios-maps-sdk to from:
"9.1.0" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1208
- chore(deps): update dependency
pointfreeco/swift-composable-architecture to from: "1.13.1" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1210
- chore(deps): update dependency nicklockwood/swiftformat to from:
"0.54.4" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1218
- chore(deps): update dependency bazel to v7.3.1 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1207
- chore(deps): update dependency iterable/swift-sdk to from: "6.5.7" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1226
- chore(deps): update dependency googlemaps/ios-maps-sdk to from:
"9.1.1" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1225
- chore(deps): update dependency nicklockwood/swiftformat to from:
"0.54.5" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1227
- chore(deps): update dependency firebase/firebase-ios-sdk to from:
"11.2.0" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1228
- chore(deps): update dependency
googlecloudplatform/recaptcha-enterprise-mobile-sdk to from: "18.6.0" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1229
- chore(deps): update dependency
pointfreeco/swift-composable-architecture to from: "1.15.0" by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1230
- chore(deps): update dependency protobuf to v27.4 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1231
- chore(deps): update dependency vapor/vapor to v4.105.2 by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1235
- chore(deps): update dependency datatheorem/trustkit to from: "3.0.4"
by
[@&#8203;cgrindel-self-hosted-renovate](https://redirect.github.com/cgrindel-self-hosted-renovate)
in
[cgrindel/rules_swift_package_manager#1148
- chore: remove legacy workspace info from release and readme by
[@&#8203;cgrindel](https://redirect.github.com/cgrindel) in
[cgrindel/rules_swift_package_manager#1237

#### New Contributors

- [@&#8203;roya1v](https://redirect.github.com/roya1v) made their first
contribution in
[cgrindel/rules_swift_package_manager#1173

**Full Changelog**:
cgrindel/rules_swift_package_manager@v0.37.0...v0.38.0

#### Bazel Module Snippet

```python
bazel_dep(name = "rules_swift_package_manager", version = "0.38.0")
```

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/bazel-contrib/rules_bazel_integration_test).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

Ambiguous copt in release mode
3 participants