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

Rename SwiftProtobuf to avoid issue #3599

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dinhnhat0401
Copy link

@dinhnhat0401 dinhnhat0401 commented Dec 14, 2023

Description

Change SwiftProtobuf lib name to avoid conflict with other Swift SPM packages.
This will solve this issue: #2088

How to test

Should have no change to functionality.

Types of changes

Change SwiftProtobuf lib name to avoid conflict with other Swift SPM packages.

Checklist

  • Create pull request as draft initially, unless its complete.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • If there is a related Issue, mention it in the description.

If you're adding a new blockchain

  • I have read the guidelines for adding a new blockchain.

@satoshiotomakan
Copy link
Collaborator

Hi @dinhnhat0401, thank you for opening the PR!
Let's see if the Swift CI passes. Then I'll discuss internally if the renamed WalletCoreSwiftProtobuf does not break backward compatibilities.

@dinhnhat0401
Copy link
Author

@satoshiotomakan thanks. I think all the CI passed (except one for not related issue)
I have tested the setting with our project and it seems working fine.

@satoshiotomakan
Copy link
Collaborator

Hi @dinhnhat0401, unfortunately, we cannot merge the PR since we do not have time to well test it.
Ideally, we should have a CI - a demo app to consume the local SPM package (with some simple tests), then we can see if that change would break our iOS app or not.
We plan to work on it the next year

@dinhnhat0401
Copy link
Author

dinhnhat0401 commented Jan 8, 2024

Happy new year.
@satoshiotomakan just want to follow up. Any update on this?

@doovers
Copy link

doovers commented Feb 12, 2024

@satoshiotomakan I'm also interested in a resolution to this. Any progress?

@satoshiotomakan
Copy link
Collaborator

Hi @doovers, @dinhnhat0401, sorry for the late reply
Unfortunately, we do not have resources at the moment to complete the testing. We track this issue, so once we finish with all our priority tasks, we'll move on this PR

@dimitris-c
Copy link

dimitris-c commented Feb 21, 2024

Hey, so this just renames the library, at runtime you'll get a warning since the system can't determine which framework to actually use.

@dinhnhat0401
Copy link
Author

@dimitris-c do you have any suggestion to make it explicitly defined?

@dimitris-c
Copy link

not off the top of my head, a lot of maintenance would be required to rename the SwiftProtobuf library for it to be unique

@dinhnhat0401
Copy link
Author

@satoshiotomakan any updates on this?

@tamimattafi
Copy link

Hello! Any updates on this?

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.

5 participants