Add "Create a List" method #7

emin-grbo · 2022-03-18T10:58:34Z

Create List method was lacking from the "Manage Lists" group.
https://developer.twitter.com/en/docs/twitter-api/lists/manage-lists/introduction

Not sure If this is the correct way to do it, but I tried to follow the same approach as for some other similar methods I saw inside Twift.

@daneden Note 👇
The main concern on my end is: "How do I test this?" 😅
So far I've only worked on personal packages and changing branches was easy 👌 However, when contributing I do not know how to test it without having to do quite a bit of duplication 😅

daneden · 2022-03-21T15:25:01Z

Sources/Twift+Tweets.swift

@@ -262,6 +262,13 @@ public struct DeleteResponse: Codable {
  public let deleted: Bool
 }

+/// A response object pertaining to list created


This type definition should probably live in Twift+Lists.swift instead of Twift+Tweets.swift

Oh wow, did not even pay attention to the file I was in. Just saw deletedResponse and thought they would fit nicely together 😂

daneden · 2022-03-21T15:25:59Z

Sources/Twift+Tweets.swift

@@ -262,6 +262,13 @@ public struct DeleteResponse: Codable {
  public let deleted: Bool
 }

+/// A response object pertaining to list created
+public struct CreatedListResponse: Codable {
+    /// If the returned response object contains an id and the name of your List, you have successfully created the List.


This doc comment will only appear when authors look up the id attribute; you should add doc comments specific to each property and add this comment as a description on the type itself

daneden · 2022-03-21T15:28:19Z

Sources/Twift+Lists.swift

+    /// - Parameters:
+    ///  - name: List name (required)
+    ///  - description: Description for the list (optional)
+    ///  - private: true or false (optional)


This is a bit of a nit, but “true or false” is not a super helpful comment when people can see the accepted type is Bool. Try to describe the meaning of the field, e.g. Whether the list is private to the authenticated user or publicly viewable

I am not used to writing these types of comments but I did pause for a second and thought if I need more context here.

It so makes sense not to mention true/false if they already see a type.
I hope to get better at this 🤞

daneden · 2022-03-21T15:29:01Z

Sources/Twift+Lists.swift

+            "description": description ?? "",
+            "private": isPrivate ?? false


Since we already have default values for description and isPrivate, you don't need to do this nullish coalescing. Just make the types for the arguments non-optional.

On second thought, making description optional does make sense

daneden · 2022-03-21T15:30:36Z

The main concern on my end is: "How do I test this?" 😅

This is a concern for me too! I don’t yet have a solid testing approach other than adding methods to a test SwiftUI app I have in a private repo. I will plan to stub a new issue specifically to set up a testing suite and strategy.

Sources/Twift+Lists.swift

daneden · 2022-03-25T10:23:54Z

Sources/Twift+Lists.swift

+    ///  - id: If the returned response object contains an id, response is considered successfull
+    ///  - name: If List name was returned, response is considered successfull


These comments should be written directly above their respective struct properties so that they show up in Xcode’s quick help and the generated GitHub wiki docs.

Oh wow, had no idea 😅

daneden · 2022-03-25T10:24:16Z

Sources/Twift+Lists.swift

+    }
+
+    /// A response object pertaining to list created
+    /// - Returns:


Nit: since this is a struct and not a function, Returns isn’t accurate and can be removed

daneden · 2022-03-25T10:25:56Z

Sources/Twift+Lists.swift

+    /// - Returns:
+    ///  - id: If the returned response object contains an id, response is considered successfull
+    ///  - name: If List name was returned, response is considered successfull
+    public struct CreatedListResponse: Codable {


This struct shouldn’t be a member of the Twift class; I’d pull it out from being nested inside the class and put it at the root of this file, along with PinnedResponse

emin-grbo · 2022-03-27T07:32:16Z

FYI, I really appreciate you being patient with all these explanations 🙌
Everything makes sense and I am learning new stuff about documentation. Never was "too" careful/knowledgeable there, mostly just "lemme plant a comment right there" 😂

I will take note of all this in the future as not to have some repeated issues 👌

Sources/Twift+Lists.swift

daneden · 2022-03-27T08:00:38Z

Sources/Twift+Lists.swift

+/// A response object pertaining to list created
+public struct CreatedListResponse: Codable {
+    /// If returned response object contains an id, response is considered successfull
+    public let id: Int


This should be typed as List.ID

daneden · 2022-03-27T08:13:14Z

Of course, thanks for your patience with the reviews and revisions! This is helpful for informing contribution guidelines too.

Co-authored-by: Daniel Eden <dan.eden@me.com>

daneden · 2022-03-27T15:49:55Z

@roblack FYI I just updated the main branch to include a demo app; you can use that to test your changes! Feel free to also provide feedback on the demo app since it’s a bit rough-and-ready (just copied from my own sandbox).

Sources/Twift+Lists.swift

Co-authored-by: Daniel Eden <dan.eden@me.com>

emin-grbo · 2022-03-27T15:55:31Z

Oh that is awesome! Will check it out and report back once tested. 🙌
I guess all future PRs should have a "did test in the sandbox app" kinda checkbox? 😅

daneden · 2022-03-27T15:58:43Z

I guess all future PRs should have a "did test in the sandbox app" kinda checkbox? 😅

Yah I’m wondering whether Xcode can drive UI tests in the demo app as a way to test library methods. I have some research to do!

emin-grbo · 2022-03-27T16:13:25Z

Cannot launch the demo app. Seems some files failed to upload?

Not sure if you want me to post these under issues, not to mix with this PR?

daneden · 2022-03-27T18:12:33Z

@roblack ah, the error for preview content may be legit but you should check the readme in the demo app folder to resolve the Secrets error 😇

emin-grbo · 2022-03-28T10:02:37Z

@daneden I kinda thought "secrets" might be something used "per user", especially after seeing some new fields in the plist file. 🤦‍♂️

I definitely need to take the blindfold off as currently I just do what the comment is and don't really "inspect" things properly. Not sure if this is making sense, but it is to me 😂 Need to be more pro-active!

daneden · 2022-03-28T17:36:21Z

@roblack I managed to fix a couple of issues in the demo app so it should build now (maybe after a clean build folder). I'm adding some custom warnings to make it easier to resolve issues too. Thanks for the feedback!

emin-grbo · 2022-03-28T18:37:48Z

@daneden HOLLY SHIZZ MAN! 🙌 That app looks/works so good, i love it!

2 notes:

Did not know .env is hidden. It was a fairly easy guess so I am sure people would manage, but my thinking is that this library will be used quite a bit by "entry level" people. I am not too advanced myself as well 😂
So, adding something like "show hidden files with cmnd+shift+." would help.
URL scheme needs to be changed as well in order for an app to work. Could this be placed in the Secrets file as well? As I am guessing Secrets is not touched by git, so changing the schemes myself would register it in the git and then possibly mess up when someone updates the repo from the origin?

Btw, the "Secrets" thing approach with the .env file is just EPIC, love it!

daneden · 2022-03-29T08:53:19Z

@roblack good catch on the URL scheme, added that to the .env.example and README.

Btw, the "Secrets" thing approach with the .env file is just EPIC, love it!

Thanks! Took me a while to figure out a safe way to simplify setup while avoiding accidentally checking in secrets 😅 still not 100% happy with it but it works.

emin-grbo · 2022-03-29T14:24:57Z

Btw I lost track if this PR is good to go?

daneden · 2022-03-29T15:12:59Z

Just tested locally and made one small change 😊 it's a small thing, but the Twitter v2 API doesn’t accept requests to paths with trailing slashes, so POST /2/lists/ results in an error, whereas POST /2/lists does not. Committed to your branch directly, so now it’s good to go!

emin-grbo · 2022-03-29T18:39:21Z

Oh wow, good to know! 🙌 Thank you!

emin added 2 commits March 18, 2022 10:59

add create list method

fcd4338

improve createList method documentation

3b301f0

emin-grbo changed the title ~~Add create a list method~~ Add "Create a List" method Mar 18, 2022

daneden requested changes Mar 21, 2022

View reviewed changes

emin added 3 commits March 21, 2022 22:42

move createdListResponse declaration to Twift+List

3b56a64

update createList method documentation

70c2772

modify parameter optionality for createList method

1bb4f7c

emin-grbo requested a review from daneden March 21, 2022 21:50

daneden reviewed Mar 25, 2022

View reviewed changes

Sources/Twift+Lists.swift Show resolved Hide resolved

daneden requested changes Mar 25, 2022

View reviewed changes

daneden mentioned this pull request Mar 26, 2022

Add missing API methods #9

Open

10 tasks

emin added 2 commits March 27, 2022 09:29

Move CreatedListResponse to root of the file

2082933

include endpoint in the method documentation

fab45d8

daneden requested changes Mar 27, 2022

View reviewed changes

Update Sources/Twift+Lists.swift

6faf791

Co-authored-by: Daniel Eden <dan.eden@me.com>

user correct type for list id

b2127c2

daneden reviewed Mar 27, 2022

View reviewed changes

Sources/Twift+Lists.swift Outdated Show resolved Hide resolved

Update Sources/Twift+Lists.swift

db9c0a0

Co-authored-by: Daniel Eden <dan.eden@me.com>

Remove trailing slash for createList route

9f648f8

daneden approved these changes Mar 29, 2022

View reviewed changes

daneden merged commit 3eac87d into daneden:main Mar 29, 2022

emin-grbo deleted the add-create-list branch June 16, 2022 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "Create a List" method #7

Add "Create a List" method #7

emin-grbo commented Mar 18, 2022

daneden Mar 21, 2022

emin-grbo Mar 21, 2022

daneden Mar 21, 2022

daneden Mar 21, 2022

emin-grbo Mar 21, 2022

daneden Mar 21, 2022

daneden Mar 21, 2022 •

edited

Loading

daneden commented Mar 21, 2022

daneden Mar 25, 2022

emin-grbo Mar 27, 2022

daneden Mar 25, 2022

daneden Mar 25, 2022

emin-grbo commented Mar 27, 2022

daneden Mar 27, 2022

daneden commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 28, 2022

daneden commented Mar 28, 2022

emin-grbo commented Mar 28, 2022

daneden commented Mar 29, 2022

emin-grbo commented Mar 29, 2022

daneden commented Mar 29, 2022

emin-grbo commented Mar 29, 2022

		"description": description ?? "",
		"private": isPrivate ?? false

		/// - id: If the returned response object contains an id, response is considered successfull
		/// - name: If List name was returned, response is considered successfull

Add "Create a List" method #7

Add "Create a List" method #7

Conversation

emin-grbo commented Mar 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daneden Mar 21, 2022 • edited Loading

Choose a reason for hiding this comment

daneden commented Mar 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emin-grbo commented Mar 27, 2022

Choose a reason for hiding this comment

daneden commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 27, 2022

daneden commented Mar 27, 2022

emin-grbo commented Mar 28, 2022

daneden commented Mar 28, 2022

emin-grbo commented Mar 28, 2022

daneden commented Mar 29, 2022

emin-grbo commented Mar 29, 2022

daneden commented Mar 29, 2022

emin-grbo commented Mar 29, 2022

daneden Mar 21, 2022 •

edited

Loading