Add List command to display a list of available exercises#154
Add List command to display a list of available exercises#154lcowell merged 1 commit intoexercism:masterfrom
Conversation
cmd/list.go
Outdated
There was a problem hiding this comment.
The fetch command actually takes two arguments: exercism fetch ruby matrix
There was a problem hiding this comment.
Good catch. It actually worked for me to say exercism fetch ruby/matrix, but we should be telling people the right thing to do, not the thing that happens to work in this release.
|
It's cool to see progress again on this project! ✨ ❤️ ✨ |
|
I think the failing test was due to a stupid mistake I made on master. |
|
One last thing I was wondering about is: should we be generating a better response when someone requests a language that doesn't exist ? eg |
|
That should return a 404, not a 500. https://github.com/exercism/x-api/issues/37 |
There was a problem hiding this comment.
I like the way this function takes separate arguments as opposed to the composed url. Even providing host seems a bit off but until we have a Client struct for the api package this is fine. I think we should follow this same pattern for the other functions in this file. Any thoughts on this?
There was a problem hiding this comment.
Thanks! I think what I like about this is that the caller of api.List doesn't need to know about how to compose a URL. AFAIK each of the api commands takes a url as an argument which means those URLs are probably being generated in each of the cmd funcs. I don't think that cmd package should know about URLs.
What do you think about making functions for generating URLs and putting them in the api package ? It kind of seems like the right place for them to me.
func ListURL(language, host string) string {}
func UnsubmitURL(host string) string {}
func FetchURL(parts ...string) string {}There was a problem hiding this comment.
Yeah, agreed. Let's let the api worry about urls (functions or hard-coded), and the function arguments can be whatever information is needed.
There was a problem hiding this comment.
That said, that is probably an unrelated refactoring.
|
The API now returns a 404 for non-existent languages. Do you want to update the PR with this before we merge? Once you're ready would you mind squashing the commits? |
* lists available exercises for a given language and explains how to fetch them * returns error for unknown languages * Resolves issue exercism#89 * Fetches the track for a specific language using the /tracks/:language endopoint on the XAPI.
|
Handling 404s is in place. Commits squashed. Wahoo! |
Add List command to display a list of available exercises
|
Sweeeet! |
Add List command to display a list of available exercises
3 participants
WIP - I just realize that I need to handle a 500 response in the case that someone tries to list for a language like 'rub' or 'haskll'.
Closes #89