Fetch current user's issues & a specific repository's issues #28

Palleas · 2016-05-24T01:15:46Z

Hello !

It's a WIP, but I figured I should open a PR just in case somebody is working on this too 😉

I'll update this soon!

RuiAAPeres · 2016-05-24T09:26:17Z

Tentacle/Issue.swift

+
+	public let locked: Bool
+	public let comments: Int
+    public let pullRequest: PullRequest?


Palleas · 2016-05-25T13:24:02Z

Slowly getting there, I'm having a hard time wrapping my head around Argo 😁

mdiep · 2016-05-26T02:09:22Z

Slowly getting there, I'm having a hard time wrapping my head around Argo 😁

Let me know if you need any help!

Palleas · 2016-05-26T02:38:28Z

Let me know if you need any help!

I think I do! I'll push my work (I've added a couple of new things, like an actually passing test) and maybe we can take things from here, I'm having a hard time with mapping some dates (updated/created and closed because the last one is nil)

Beside that I think it should be ready quickly because most of the work was already there. :)

Palleas · 2016-05-26T13:43:58Z

Tentacle/Issue.swift

+//			<*> j <| "milestone"
+			<*> j <| "locked"
+			<*> j <| "comments"
+//			<*> (j <|? "closed_at" >>- toNSDate)


@mdiep do you see something wrong with this ? Somehow it fails at mapping the date, saying the keys closed_at, updated_at and created at don't exist when they do (but the closed_at is null).

I've updated the decode() method temporarily to print the error when mapping fails)

When I run the test you added with the fixture data, the createdAt and updatedAt fields are correctly decoded. 😕

(Sorry I didn't look at this sooner!)

Yeah it seems to be working, but it fails because of a "MissingKey" error, see the log:

Returning date: 2016-05-24 23:38:39 +0000 Returning date: 2016-05-24 23:38:39 +0000 decoded = Failure(MissingKey(created_at))

Am I missing something?

Oooooh, I think the created_at error does not come from the Issue but probably something else!

When I remove the mapping User part by commenting out the user argument, it works. Could this be a bug in Argo..? I read the relationship were supposed to be working auto-magically... 😁

Yeah! That makes it easier to review and discuss. 😄

Are you still interested in pursuing this?

I just come back from vacations, sorry I should have warned you. I'm still interested in working on this, but if you think you can tackle the user refactoring part relatively soon, it'll probably be better since I'm finding my ways in Tentacle. 😄

This sounded like a nice break from reviewing PRs, so I did it tonight: #31. 😄

Cool! I'll rebase my branch and keep working on this this week! Thanks 😋

Palleas · 2016-07-08T19:17:32Z

I'll dig a little more during the week-end but I'm slowly getting there. 😄

Palleas · 2016-07-09T01:06:51Z

@mdiep I'm sorry but I have one more issue with Argo / Curry. In the current Issue I'm using in my test, the "closed_at" is null, which means the tests fail with MissingKey(closed_at). The test passes when I set a valid date for the closed_at key in the payload.
Is this because the toNSDate function doesn't accept an optional String? I've tried using <|? instead of just <| but it doesn't work.

Otherwise I've started using Tentacle to fetch Issues in my app and it works well 👌

Palleas · 2016-07-14T02:02:06Z

@mdiep I just added support for Milestone and PullRequest so I think this is ready for a deeper code review. Let me know if you want me to squash my branch first!

mdiep · 2016-07-14T23:32:04Z

Tentacle.xcodeproj/project.pbxproj

+				7A1A82551CF3DBAC0076E2DD /* Issue.swift */,
+				7A1A82571CF3DE4C0076E2DD /* Label.swift */,
+				7A1A82591CF3DEEA0076E2DD /* Milestone.swift */,
+				7A1A825D1CF3E5B60076E2DD /* PullRequest.swift */,


Could you alphabetize the new files in the Xcode project navigator? 🙏

mdiep · 2016-07-18T02:17:55Z

Tentacle/Issue.swift

+    public let id: Int
+
+    /// The URL to view this issue in a browser
+    public let url: NSURL?


This should be URL.

mdiep · 2016-07-18T02:26:10Z

For the label colors, I've simply copied the code from SwiftHEXcolors (keeping the license ofc) because it seemed a bit overkill to add it as a Carthage dependency and a bit useless to write my own poorer implementation of Hex color parsing. Please let me know what you think.

I'm not wild about doing that. Let me think about it.

Palleas · 2016-07-18T13:47:14Z

I'm not wild about doing that. Let me think about it.

Sure, let me know 😄

mdiep · 2016-07-19T12:39:50Z

Tentacle/Issue.swift

+    && lhs.updatedAt == rhs.updatedAt
+    && lhs.labels == rhs.labels
+    && lhs.milestone == rhs.milestone
+    && lhs.pullRequest == rhs.pullRequest


Indentation is off here. && lines should be indented another level.

mdiep · 2016-07-19T13:01:17Z

Here's a Color initializer that we can use:

import AppKit

typealias Color = NSColor

extension Color {
    convenience init(hex: String) {
        precondition(hex.characters.count == 6)

        let scanner = NSScanner(string: hex)
        var rgb: UInt32 = 0
        scanner.scanHexInt(&rgb)

        let r = CGFloat((rgb & 0xff0000) >> 16) / 255.0
        let g = CGFloat((rgb & 0x00ff00) >>  8) / 255.0
        let b = CGFloat((rgb & 0x0000ff) >>  0) / 255.0
        self.init(red: r, green: g, blue: b, alpha: 1)
    }
}

(You'll need to add some #ifs to make it work on iOS too.)

Would you mind adding some tests for it? These cases should be a decent start:

Color(hex: "ffffff")
Color(hex: "aa0000")
Color(hex: "00aa00")
Color(hex: "0000aa")
Color(hex: "000000")

Palleas · 2016-07-19T16:08:57Z

Done, done, done and done. Slowly getting there!

mdiep · 2016-07-20T00:28:58Z

This is great! Thank you for all the work you put in on this! ⚡ ⚡ ⚡

Palleas · 2016-07-20T01:31:04Z

Wouhou, cool! 🎉

Palleas force-pushed the user-issues branch from 183fc12 to 08d4000 Compare May 24, 2016 01:32

RuiAAPeres reviewed May 24, 2016
View reviewed changes

Tentacle/Issue.swift

public let locked: Bool

public let comments: Int

public let pullRequest: PullRequest?

Copy link

RuiAAPeres May 24, 2016

Choose a reason for hiding this comment

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

Identation

Palleas reviewed May 26, 2016
View reviewed changes

Palleas force-pushed the user-issues branch from a891d35 to 8dd0d7b Compare July 8, 2016 18:59

Palleas changed the title ~~[WIP] Add method to fetch current user's issues~~ Add method to fetch current user's issues Jul 14, 2016

Palleas changed the title ~~Add method to fetch current user's issues~~ Fetch current user's issues & a specific repository's issues Jul 14, 2016

mdiep reviewed Jul 14, 2016
View reviewed changes

mdiep reviewed Jul 18, 2016
View reviewed changes

Tentacle/Issue.swift

public let id: Int

/// The URL to view this issue in a browser

public let url: NSURL?

Copy link

Owner

mdiep Jul 18, 2016

Choose a reason for hiding this comment

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

This should be URL.

Palleas added 5 commits July 18, 2016 09:26

Issues → AssignedIssues

d96630c

Rename url → URL and fix indent

71a9e44

Remove dup file

341353e

Use assignees list

d68bf73

Reindent

f331218

mdiep reviewed Jul 19, 2016
View reviewed changes

Palleas added 6 commits July 19, 2016 10:22

More ident fixes

74b091b

contentDescription -> body

0762aec

Make milestone due date optional

b483033

Add doc in Milestone

500f8bd

Rename count of things to match Swift's stdlib

4d99b2e

Add custom color type

3b99f04

mdiep merged commit 1b5b018 into mdiep:master Jul 20, 2016

Palleas deleted the user-issues branch July 20, 2016 01:30

mdiep mentioned this pull request Jul 20, 2016

Fix submodules to be in sync with Cartfile.resolved #33

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fetch current user's issues & a specific repository's issues #28

Fetch current user's issues & a specific repository's issues #28

Palleas commented May 24, 2016

RuiAAPeres May 24, 2016

Palleas commented May 25, 2016

mdiep commented May 26, 2016

Palleas commented May 26, 2016

Palleas May 26, 2016

mdiep Jun 3, 2016

Palleas Jun 7, 2016

Palleas Jun 7, 2016 •

edited

Loading

Palleas Jun 7, 2016

mdiep Jun 7, 2016

mdiep Jul 4, 2016

Palleas Jul 4, 2016 •

edited

Loading

mdiep Jul 5, 2016

Palleas Jul 5, 2016

Palleas commented Jul 8, 2016

Palleas commented Jul 9, 2016

Palleas commented Jul 14, 2016

mdiep Jul 14, 2016

mdiep Jul 18, 2016

mdiep commented Jul 18, 2016

Palleas commented Jul 18, 2016

mdiep Jul 19, 2016

mdiep commented Jul 19, 2016 •

edited

Loading

Palleas commented Jul 19, 2016

mdiep commented Jul 20, 2016

Palleas commented Jul 20, 2016

Fetch current user's issues & a specific repository's issues #28

Fetch current user's issues & a specific repository's issues #28

Conversation

Palleas commented May 24, 2016

Choose a reason for hiding this comment

Palleas commented May 25, 2016

mdiep commented May 26, 2016

Palleas commented May 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Palleas Jun 7, 2016 • edited Loading

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

Palleas Jul 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Palleas commented Jul 8, 2016

Palleas commented Jul 9, 2016

Palleas commented Jul 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdiep commented Jul 18, 2016

Palleas commented Jul 18, 2016

Choose a reason for hiding this comment

mdiep commented Jul 19, 2016 • edited Loading

Palleas commented Jul 19, 2016

mdiep commented Jul 20, 2016

Palleas commented Jul 20, 2016

Palleas Jun 7, 2016 •

edited

Loading

Palleas Jul 4, 2016 •

edited

Loading

mdiep commented Jul 19, 2016 •

edited

Loading