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

Tests for GithubCommands module #83

Merged
merged 8 commits into from
May 24, 2019

Conversation

HoffsMH
Copy link
Contributor

@HoffsMH HoffsMH commented May 17, 2019

Connected to #48, according to my coveralls this puts coverage at 57%

Also fixes a couple bugs in the module which I will leave comments next to
I took the liberty of trying to converge on a common spelling of reusable if there is a preferred alternate spelling let me know and I can rename.

If this is too hefty I can split out into separate PRs as well!

def create_reusable_stories(
%{project_name: project_name} = results,
%{project_name: project_name, github_repo: _} = results,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

requiring this field in the results map allows us to skip the step if there is no github_repo to post reusable stories to

end
end

def create_reusable_stories(results, _, _, _, _), do: results
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default case when no github_repo from a previous step exist is now to just return the results map

@@ -138,7 +151,7 @@ defmodule Slax.Commands.GithubCommands do
end)
|> Enum.split_with(fn
{:ok, _, _} -> true
{:error, _} -> false
{:error, _, _} -> false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the previous Enum.map was generating either 2-element :ok tuples or 3-element :error tuples but split_with was matching on a 2-element :error tuple here. This resulted in split_with raising an error any time there was an error creating an issue.

Copy link
Contributor

@bryanjos bryanjos left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for fixing the typo and the bug!

@bryanjos
Copy link
Contributor

@HoffsMH looks like there is a conflict

HoffsMH added 8 commits May 24, 2019 10:40
including error stats when various requests fail or formatting is incorrect
…ons or change module names when refactoring
…e_stories

and the result set doesnt include a github repo, which means that even if we were
to parse out the story issues correctly there would be nothing to send them to.
a github repo should be a hard requirement for this step
@HoffsMH HoffsMH force-pushed the github-commands-tests branch from 6a0eced to e9863f1 Compare May 24, 2019 15:42
@HoffsMH
Copy link
Contributor Author

HoffsMH commented May 24, 2019

@bryanjos Rebased!

@bryanjos bryanjos merged commit b91ee30 into revelrylabs:master May 24, 2019
@bryanjos
Copy link
Contributor

Thanks!

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