Skip to content
This repository has been archived by the owner on Jun 8, 2019. It is now read-only.

Backward compatibility (username vs. login) #28

Merged
merged 1 commit into from
Dec 16, 2016
Merged

Backward compatibility (username vs. login) #28

merged 1 commit into from
Dec 16, 2016

Conversation

martinhpedersen
Copy link

The rename from username to login broke compatibility with Drone and other API consumers.

This PR adds the old username field to User on json.Marshal, which resolves these compatibility issues while still maintaining Github-API compliance.

The patch has been tested against Drone 0.4.

Reintroduce the username field for user object on JSON marshal to fix backward
compatibility. Fixes Drone 0.4 compatibility issues.
@tboerger
Copy link
Member

Something like this I have been talking about :)

@strk
Copy link
Member

strk commented Dec 15, 2016

LGTM

@bkcsoft
Copy link
Member

bkcsoft commented Dec 15, 2016

I'm not entirely sure how this would fix anything? 😕

@martinhpedersen
Copy link
Author

Hi - so I guess some more explanation are in order :)

While cc453bf fixed the issue of incompatible push webhook payloads (gogs/gogs#3653), it also changed other parts of the API.

For instance, Drone 0.4 expects /api/v1/user/repos to return the username attribute (under owner) which has now been renamed to login.

This PR resolves these issues by marshalling the UserName field as both login and username in the JSON structure, thus ensuring backward compatibility with clients expecting the old username attribute.

Currently (without this patch) Drone 0.4 is unable to determine the owner of a repo, and in turn is unable to communicate with the gitea API.

See also @richmahn's comment in PR #9.

@bkcsoft
Copy link
Member

bkcsoft commented Dec 16, 2016

@martinhpedersen Aaah! Now I get how it works 👍

LGTM

@bkcsoft bkcsoft merged commit 76837c0 into go-gitea:master Dec 16, 2016
@martinhpedersen
Copy link
Author

Excellent :) Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants