Skip to content

Get more information from gitlab #186

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

Merged
merged 10 commits into from
May 25, 2018
Merged

Get more information from gitlab #186

merged 10 commits into from
May 25, 2018

Conversation

ISibboI
Copy link
Contributor

@ISibboI ISibboI commented May 24, 2018

Hey,

as mentioned in issue #180 before, here is a pull request with additions to the API. I added more than just the one missing method in this. I hope it's fine to merge it alltogether.

Additions:

  • get commit comments
  • get merge request changes: basically a diff of the merge request
  • get merge request participants
  • get project forks
  • get repository contributors

Fixes:

  • getFile method encoded spaces in file paths as + and not %F0, as gitlab wants.
  • Two javadoc-formatted comments were not marked as javadoc comments

Copy link
Collaborator

@gmessner gmessner left a comment

Choose a reason for hiding this comment

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

@ISibboI
Thank you for your contribution. I've requested a couple small changes, but as a whole, it looks great.

* @throws GitLabApiException if encoding throws an exception
*/
protected String urlEncodeForPath(String s) throws GitLabApiException {
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just add encoded = encoded.replace("+", "%20"); to urlEncode(String s)

I'm pretty sure urlEncode(String s) not processing + is an oversight

@@ -43,7 +43,7 @@ public RepositoryFile getFile(String filePath, Integer projectId, String ref) th
Form form = new Form();
addFormParam(form, "ref", ref, true);
Response response = get(Response.Status.OK, form.asMap(),
"projects", projectId, "repository", "files", urlEncode(filePath));
"projects", projectId, "repository", "files", urlEncodeForPath(filePath));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change back to urlEncode(filePath) after fix to AbstractApi.urlEncode(String s)

@gmessner
Copy link
Collaborator

gmessner commented May 25, 2018

After further thought and examination of the other classes that sub-classed AbstractUser, I have changed my mind and deleted the comments about not needing Contributor and Participant classes. I now believe it is correct creating those classes. Address the urlEncode() change and this is good to merge.

Put functionality of AbstractApi.urlEncodeForPath into
AbstractApi.urlEncode and removed the former.
@ISibboI
Copy link
Contributor Author

ISibboI commented May 25, 2018

Thank you for your feedback! I was not sure what the urlEncode method exactly does, so I first implemented a second one, to be safe. Now it should be as you requested.

@gmessner gmessner merged commit b96fd36 into gitlab4j:master May 25, 2018
@gmessner
Copy link
Collaborator

@ISibboI
Thanks again for your contribution. I've merged and will do a release later today.

@gmessner
Copy link
Collaborator

@ISibboI
This is now available n release 4.8.20

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

Successfully merging this pull request may close these issues.

2 participants