Skip to content
This repository was archived by the owner on Dec 10, 2024. It is now read-only.

Fix ISOTime encoding in GET requests #554

Conversation

sirlatrom
Copy link
Contributor

Implement the query.Encoder interface on ISOTime, ensuring correct encoding for GET requests, and add url struct tags to structs in events.go to ensure correct parameter names.

Also implement the query.Encoder interface on ISOTime.

Signed-off-by: Sune Keller <absukl@almbrand.dk>
Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

This is indeed the correct solution, thanks @sirlatrom! But do have 2 small comments that I would like to address.

events.go Outdated
AvatarURL string `url:"avatar_url" json:"avatar_url"`
WebURL string `url:"web_url" json:"web_url"`
} `url:"author" json:"author"`
AuthorUsername string `url:"author_username" json:"author_username"`
Copy link
Member

Choose a reason for hiding this comment

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

This struct is never used in a GET call, so why would we want to add the url tags here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's just a result of me being lazy with a regexp replace all in my editor; I'll revert that part in my next push.

Removes `url` struct tags from ContributionEvent and combines two checks
in one line.

Signed-off-by: Sune Keller <absukl@almbrand.dk>
@svanharmelen
Copy link
Member

LGTM! Thanks @sirlatrom!

@svanharmelen svanharmelen merged commit cd1e3f0 into xanzy:master Jan 30, 2019
@sirlatrom sirlatrom deleted the contribution-events-before-after-isotime-fix branch January 30, 2019 12:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants