Skip to content
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

[Documentation] Write documentation for Jenkiexs.Builds.details/2 #18

Closed
GPrimola opened this issue Sep 24, 2020 · 3 comments · Fixed by #37
Closed

[Documentation] Write documentation for Jenkiexs.Builds.details/2 #18

GPrimola opened this issue Sep 24, 2020 · 3 comments · Fixed by #37
Labels
documentation Improvements or additions to documentation Hacktoberfest help wanted Extra attention is needed

Comments

@GPrimola
Copy link
Owner

GPrimola commented Sep 24, 2020

Please, follow this guidelines

@GPrimola GPrimola added documentation Improvements or additions to documentation help wanted Extra attention is needed Hacktoberfest labels Sep 24, 2020
@GPrimola GPrimola changed the title [Documentation] Make documentation for Jenkiexs.Builds.details/2 [Documentation] Write documentation for Jenkiexs.Builds.details/2 Sep 25, 2020
@GPrimola
Copy link
Owner Author

GPrimola commented Oct 2, 2020

PS: an important highlight is the Doctests session, as it's part of the documentation!

@dflima
Copy link
Contributor

dflima commented Oct 27, 2020

I noticed something in Jenkiexs.Buids.details/2 but didn't want to change in the abovementioned PR because it's not its purpose.

Jenkiexs.Builds.details/2 uses Client.get!/3 as follows:

  def details(job_name, build_number) do
    case Client.get!("/job/#{job_name}/#{build_number}/api/json") do
      %{status_code: 200, body: body} ->
        build = new(job_name, body)
        {:ok, build}

      %{status_code: status, body: body} ->
        {:error, "Got status #{status} with body #{inspect(body)}."}
    end
  end

What do you think about changing that request to Client.get/3 and then creating a Jenkiexs.Builds.details!/2 just to keep the API consistent?

@GPrimola
Copy link
Owner Author

@dflima thank you for noticing that! I just opened a PR for this issue!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation Hacktoberfest help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants