search

Automattic / Gravatar-SDK-iOS

Gravatar SDK is a Swift library that allows you to integrate Gravatar features into your own iOS applications.
https://gravatar.com
Mozilla Public License 2.0
41 stars 5 forks source link

Prep work for adding support to Enterprise distribution via App Center #326

Closed mokagio closed 3 months ago

mokagio commented 3 months ago

Description

Restructures the project settings and adds some Fastlane tooling to enable Enterprise distribution via App Center.

Unfortunately, I had to update the source code to make certain type public in order to build the two apps for distribution. See b6610db @andrewdmontgomery @pinarol @etoledom I don't pretend to know what's best from an API design point of view in terms of access control etc, so I'll leave it up to you to adapt the code to fit your plans and keep distribution possible. I would strongly encourage against abusing @testable to access non public types outside of test targets, though.

Testing Steps

Notice the green CI, which verifies the infra and coding changes have not broken the demo apps in Debug mode.

For Enterprise distribution, try running make build-demo-for-distribution.

andrewdmontgomery commented 3 months ago

@pinarol @etoledom

I wonder if we want to rename Gravatar-Demo:

This would involve renaming/updating:

The reason I dropped Gravatar above is that this would also update the PRODUCT_NAME. And as it stands, when the app gets installed, the name is getting truncated: Gravatar-Sw...

Here's a draft PR up with these changes, targeting this branch, so you can see what it would look like: #327

etoledom commented 3 months ago

I'm OK editing the names.

  • Swift-Demo
  • SwiftUI-Demo

Since the difference is the framework maybe it could be:

andrewdmontgomery commented 3 months ago

Since the difference is the framework maybe it could be:

  • UIKit-Demo
  • SwiftUI-Demo

I like that. I'll update the PR.

andrewdmontgomery commented 3 months ago

@mokagio asked something in the PR I made with the rename:

https://github.com/Automattic/Gravatar-SDK-iOS/pull/327#pullrequestreview-2197856842

Have you considered renaming the folders as well? I think if the rename is dove via Xcode, all the reference in the project file will update automatically.

I did. In fact, I did rename the Gravatar-Demo group/folder to Gravatar-UIKit-Demo. It does work they way you describe. The only thing that needs to be manually fixed is the info.plist File Build Setting for each target (since the folder name is in the path).

As I mentioned above:

(for these, we could keep the Gravatar- prefix... or not)

I'm good either way. It's entirely up to us, @etoledom and @pinarol .

andrewdmontgomery commented 3 months ago

That PR got merged before we could test it. So we should test building the demo apps in this PR (with Xcode and make). I tested, obviously. But someone else should, too.

Also, @pinarol , this got merged before you had a chance to see it. So chime in. Any concerns, we can revert.

One thing I noticed: I forgot to update the Makefile with the new scheme names. I was about to update my branch when I noticed you merged it. So I can just push a commit here with that, if that's alright.

mokagio commented 3 months ago

So I can just push a commit here with that, if that's alright.

@andrewdmontgomery all good, and thanks for following up!

pinarol commented 3 months ago

The reason I dropped Gravatar above is that this would also update the PRODUCT_NAME. And as it stands, when the app gets installed, the name is getting truncated: Gravatar-Sw...

Also, @pinarol , this got merged before you had a chance to see it. So chime in. Any concerns, we can revert.

@andrewdmontgomery

I am confused because from now on when the app installed there will be no "Gravatar" in the name. Wouldn't it be more confusing for non-developers who will install the app via AppCenter? I would have difficult time finding the app in my real device among many other icons and my first intuition wouldn't be searching via the "Swift" keyword. The name should be something easily findable via spotlight search with "Gravatar" keyword. So I think we should keep "Gravatar" in the name even though it gets a bit truncated from the end or we can come up with sth short but still keep it.

Just updating the "Display Name" field from XCode should be sufficient I think. How does Gravatar UIKit, Gravatar SwiftUI sound? We don't get truncation this way but even if it gets truncated from the end a bit, this is much more clear for a non-dev:

Screenshot 2024-07-25 at 10 05 10

Also, we don't need to emphasize "Swift" in the name or in the icon. Almost every app is in Swift now, it is not a distinctive thing. We need to emphasize "Gravatar" in the name and the distinctive part is whether it is UIKit or SwiftUI.

We can maybe emphasize that this is a demo app with a suitable icon like combining the Gravatar icon with a construction item. 🤔

mokagio commented 3 months ago

Unfortunately, I had to update the source code to make certain type public in order to build the two apps for distribution. See b6610db @andrewdmontgomery @pinarol @etoledom I don't pretend to know what's best from an API design point of view in terms of access control etc, so I'll leave it up to you to adapt the code to fit your plans and keep distribution possible. I would strongly encourage against abusing @testable to access non public types outside of test targets, though.

Also, this PR is almost done and I'm keen on your feedback. It's still in draft because I need to verify something about the MATCH_PASSWORD env var behavior.

etoledom commented 3 months ago

Here is a fast PR updating the icons of the demo apps:

https://github.com/Automattic/Gravatar-SDK-iOS/pull/328

pinarol commented 3 months ago

Unfortunately, I had to update the source code to make certain type public in order to build the two apps for distribution.

Thanks a lot. I did a rearrangement on the access modifiers since we don't want to make some of them public. https://github.com/Automattic/Gravatar-SDK-iOS/pull/326/commits/2dd26e8de650d221d8767b8db5167d7b33057696

andrewdmontgomery commented 3 months ago

@pinarol you make some great points above. I was thinking of this from within our own context: getting builds to our designer. But you are right.

I like the Gravatar-SwfitUI and Gravatar-UIKit names. I concidered them, too. The problem I was trying to avoid is that we will likely have a Gravatar mobile app. And this will not be related.

Still, I think either way, there will be some ambiguity, and the names you've suggested deal much better with the existing problem: recognizable demo app names.

Just updating the "Display Name" field from XCode should be sufficient I think. How does Gravatar UIKit, Gravatar SwiftUI sound?

I suggest that we update the target and scheme names, too. Consistency is beneficial here:

I'll put up a quick PR for that

andrewdmontgomery commented 3 months ago

I suggest that we update the target and scheme names, too. Consistency is beneficial here:

Target name == Scheme name == Product name I'll put up a quick PR for that

329

Take a look, comment, etc.

Note that I split the difference a little. Having the target name match the product name I think is an intuitive connection:

The scheme and folder names (and the .xcconfig files) can be more verbose, and I think that verbosity is pretty nice: Gravatar-SwiftUI-Demo reads nicely.

So we have good consistency, with concise and intuitive target/product names.

At least that's my pitch. Chime in on that PR with thoughts.

mokagio commented 3 months ago

I did a rearrangement on the access modifiers since we don't want to make some of them public. https://github.com/Automattic/Gravatar-SDK-iOS/commit/2dd26e8de650d221d8767b8db5167d7b33057696

@pinarol thank you for that. I hope my tweaks were decent enough to build on top of without too much hassle.

pinarol commented 3 months ago

@pinarol thank you for that. I hope my tweaks were decent enough to build on top of without too much hassle.

No worries at all. I am glad you made the necessary changes instead of getting blocked. 👍