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

GravatarUI docc bundle #229

Merged
merged 6 commits into from
Apr 30, 2024
Merged

GravatarUI docc bundle #229

merged 6 commits into from
Apr 30, 2024

Conversation

etoledom
Copy link
Contributor

Closes #155

Description

This PR implements the basics to add docs and pages to GravatarUI using Swfit-docc.

I've added a new snapshot test case to generate an example profile view to use in the docs. For now I've decided to use the same account that is used in https://docs.gravatar.com/general/profile/ , though this this is subject to change. More important than the account itself, is to check the strategy chosen to generate and use these snapshots.

I've created a make command to copy the snapshots from the test target to the docs bundle:
make update-example-snapshots

This is not hook to anything yet, so it need to be triggered manually.
Sadly in SPM we don't have a way to trigger post-test commands.
We can add a post-action to the test target in the Xcode scheme, but it's a local setting.

Testing Steps

  • Checkout the banch
  • Build the project
  • On Xcode, go to Product -> Build Documentation
  • Though the content is also subject to change, is enough to see that the main GravatarUI page renders correctly.

CleanShot 2024-04-24 at 18 33 32@2x

@etoledom etoledom added the documentation Improvements or additions to documentation label Apr 24, 2024
@etoledom etoledom requested a review from pinarol April 24, 2024 16:41
@etoledom etoledom self-assigned this Apr 24, 2024
@maxme
Copy link
Contributor

maxme commented Apr 25, 2024

I remember Beau asking somewhere to be removed from https://docs.gravatar.com/general/images/, might be worth changing the default screenshots here.

// 3. Get the instance of a ProfileView:
let profileView = ProfileView()
// 4. Set the profile to the view:
profileView.update(with: profile)
Copy link
Contributor

@maxme maxme Apr 25, 2024

Choose a reason for hiding this comment

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

Kind of related to this documentation, as a newbie Swift dev, I'd like to know how you handle errors here :)

We're changing the network API on Android and had a discussion about errors on this PR: https://github.com/Automattic/Gravatar-SDK-Android/pull/130/files#r1579020468

Copy link
Contributor Author

@etoledom etoledom Apr 25, 2024

Choose a reason for hiding this comment

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

We will have some more-in-detail implementation examples in other pages. I plan a Getting Started page for example, and a tutorial for making lists of profile cards. Also probably another for the presentation styles.

This one is meant to be only introductory 👍

But still from this example, if someone copy paste this code, error handling will be enforced by the compiler from the try clause. So, though it's not explicit, it won't be missed.

let profile = try await service.fetch(with: .email("user@email.com"))

Copy link
Contributor

Choose a reason for hiding this comment

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

In Swift all errors are like checked exceptions so the compiler forces you to handle them. However, they are not declared in the method signature(you only add "throws"), so if you want to handle a specific error you need to do a type casting first. :( And you never know what sorts of errors will be thrown just by looking at the signature. So to compensate for that we try to keep a catalog of errors that we are throwing, that way 3rd party devs can have an idea.

https://github.com/Automattic/Gravatar-SDK-iOS/blob/8dd5a4a4a48e7966514b627cc8bc428df5417828/Sources/Gravatar/Network/Errors.swift

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thanks for the explanation!

@etoledom
Copy link
Contributor Author

I remember Beau asking somewhere to be removed from https://docs.gravatar.com/general/images/, might be worth changing the default screenshots here.

Oh that's good to know.
We were talking with @pinarol about this also. We don't really want to keep this example account either. I just couldn't decide on which another image/information (even from a random generator 😅 )

But yes, the idea is to change it for an account of a not-real person.

@pinarol
Copy link
Contributor

pinarol commented Apr 30, 2024

I think we can generate one from random generator to unblock this PR. It would be good to have these changes in trunk since we might want to add more docs at the top of this. We can always update our snapshots in the future. WDYT? @etoledom

@etoledom
Copy link
Contributor Author

I think we can generate one from random generator to unblock this PR. It would be good to have these changes in trunk since we might want to add more docs at the top of this. We can always update our snapshots in the future. WDYT? @etoledom

Cool! Let's do that

@etoledom
Copy link
Contributor Author

I've chosen an AI generated image and description, plus personal information from the simulator's contact app.

Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@etoledom etoledom merged commit bbda3a3 into trunk Apr 30, 2024
6 checks passed
@etoledom etoledom deleted the etoledom/gravatarui-docs branch April 30, 2024 10:46
@pinarol
Copy link
Contributor

pinarol commented Apr 30, 2024

personal information from the simulator's contact app

Good choice. This is pretty familiar for any iOS dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure documentation for the Gravatar & GravatarUI split
3 participants