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

Clean up API package. #157

Merged
merged 2 commits into from
Jan 23, 2015
Merged

Clean up API package. #157

merged 2 commits into from
Jan 23, 2015

Conversation

Tonkpils
Copy link
Contributor

Use an API Client from the cmd package
Have functions form up their own urls and give them parameters they need

I wanted to get this proof of concept here to get some feedback. Meanwhile, I'll write some tests around this new structure.

Use an API Client from the cmd package
Have functions form up their own urls and give them parameters they need

UnknownLanguageError = errors.New("the language is unknown")
// ErrUnknownLanguage represents an error returned when the language requested does not exist
ErrUnknownLanguage = errors.New("the language is unknown")
Copy link
Member

Choose a reason for hiding this comment

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

Much more idiomatic!

@kytrinyx
Copy link
Member

This looks much, much cleaner. I would say go for it.

Tonkpils added a commit that referenced this pull request Jan 23, 2015
@Tonkpils Tonkpils merged commit 400f507 into exercism:master Jan 23, 2015
@Tonkpils
Copy link
Contributor Author

I'll keep adding tests as I go but the main idea is there.

@Tonkpils Tonkpils deleted the api-client branch January 23, 2015 21:20
@lcowell
Copy link
Contributor

lcowell commented Jan 23, 2015

Awesome! Don't forget to give yourself some credit in the changelog.

@kytrinyx
Copy link
Member

I've added an entry to the changelog for this. Thanks!

Tonkpils added a commit that referenced this pull request Jan 28, 2015
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.

3 participants