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

Updating package definition, minimum platform versions as well as dependencies #161

Merged
merged 7 commits into from
Apr 25, 2024

Conversation

Muhieddine-El-Kaissi
Copy link
Contributor

@Muhieddine-El-Kaissi Muhieddine-El-Kaissi commented Apr 25, 2024

  1. Updating package definition to 5.10
  2. Updating minimum platform versions to iOS 15 & OS 14
  3. Updating swift version to 5.10
  4. Cleaning up RXswift lint
  5. Updating swiftlint & swift format as well as applying fixes to the code

@Muhieddine-El-Kaissi Muhieddine-El-Kaissi changed the title Updating package definition to 5.10 + minimum platform versions to iOS 15 & OS 14 Updating package definition, minimum platform versions as well as dependencies Apr 25, 2024
steps:
- name: 📥 Checkout
uses: actions/checkout@v2

- name: Select Xcode
run: sudo xcode-select -s "/Applications/Xcode_15.3.app"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you can do this ? These are githubs machines, not ours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's the work around proposed in https://stackoverflow.com/a/73896628/695599 and it fixed the lint issue we are seeing previously

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's very strange to me that you can run sudo at all here 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah we need sudo and it's proposed by the actions :)
Screenshot 2024-04-25 at 12 12 07 PM

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's not proposed by the actions, but by xcode-select. It doesn't know it's run from github.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

The vms aren't dedicated though, so i think you'd run into it again. I think eventually you'd be able to drop it when 5.10 is the default they have etc.

Copy link
Contributor

@dwroth dwroth left a comment

Choose a reason for hiding this comment

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

Code changes look good to me. I don't have much context on the CI stuff, so I'll let Scott approve.

@Muhieddine-El-Kaissi Muhieddine-El-Kaissi merged commit 014205c into main Apr 25, 2024
3 checks passed
@Muhieddine-El-Kaissi Muhieddine-El-Kaissi deleted the m/upgrade branch April 25, 2024 21:51
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.

None yet

3 participants