Skip to content

Update with pipeline schedule support #319

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 6 commits into from
Mar 19, 2019
Merged

Update with pipeline schedule support #319

merged 6 commits into from
Mar 19, 2019

Conversation

lpiet
Copy link
Contributor

@lpiet lpiet commented Mar 11, 2019

Hopefully doing this the right way, but I figured if I open the issue I might contribute to the solution.

Tested it against our on premise gitlab instance and it seems to work. Hopefully should help implement the support for the pipeline schedules.

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.

@lpiet
The PR looks great. Just 3 small change requests:

  1. Move the new pipeline schedule methods from ProjectApi to PipelineApi.

  2. Create a pipeline-schedule.json file in src/test/resources/org/gitlab4j/api and add a test for it to src/test/java/org/gitlab4j/api/TestGitLabApiBeans.java. You'll find the JSON at: https://docs.gitlab.com/ee/api/pipeline_schedules.html#get-all-pipeline-schedules

  3. Create src/test/java/org/gitlab4j/api/TestPipelineApi.java and move the pipeline schedule tests from TestProjectApi.java into it. I will add additional Pipeline API tests to this in the future.

@@ -2395,4 +2395,166 @@ public Project setProjectAvatar(Object projectIdOrPath, File avatarFile) throws
Response response = putUpload(Response.Status.OK, "avatar", avatarFile, "projects", getProjectIdOrPath(projectIdOrPath));
return (response.readEntity(Project.class));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The pipeline schedule methods should be moved to the org.gitlab4j.api.PipelineApi class

@lpiet
Copy link
Contributor Author

lpiet commented Mar 15, 2019

@gmessner,

I believe this covers it. I did notice that if I copy the exact json from the site any timestamp with a millisecond field of 000 fails the test (if it has any millisecond value it works) . But this was also the case for the existing tests.

The json converter strips the trailing 0's and then the generated and input json don't match.

ex. "next_run_at": "2017-05-19T13:41:00.000Z".

PS
I'll have one or two extra pull requests up next week, working through the functions I need for the role out at work and adding where needed.

@gmessner
Copy link
Collaborator

@lpiet
There are caveats when testing the JSON, this being one of them, I should have mentioned this, my apologies if you spent much time on this. The other big caveat is that values in the test file cannot have null as a value.

@gmessner
Copy link
Collaborator

@lpiet
I am assuming that you are done with this PR?

@lpiet
Copy link
Contributor Author

lpiet commented Mar 18, 2019

correct

@gmessner gmessner merged commit 88ec32b into gitlab4j:master Mar 19, 2019
@gmessner
Copy link
Collaborator

@lpiet
This has been merged, A release will happen later this week. WIll let you know when the release is completed.

Thanks again for your contribution.

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