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

Move language track icons into track repositories #2925

Closed
kytrinyx opened this issue Jun 1, 2016 · 17 comments
Closed

Move language track icons into track repositories #2925

kytrinyx opened this issue Jun 1, 2016 · 17 comments

Comments

@kytrinyx
Copy link
Member

kytrinyx commented Jun 1, 2016

It doesn't make sense to keep track-specific content in the main repository. It would be much easier to manage if this were in the respective language track repositories.

I suggest that we:

  • Add a directory named img/ in each track repository at the root level (if it doesn't already exist).
  • Put the icon in here as icon.png and optionally icon.svg
  • Serve the track icon from x-api
  • Provide the URL to the track icon in the basic track data that x-api serves up
@joeltaylor
Copy link

On it!

I imagine that this should also be added to future course checklists?

@kytrinyx
Copy link
Member Author

kytrinyx commented Jun 9, 2016

Yepp! In here somewhere https://github.com/exercism/x-template

@joeltaylor
Copy link

Encountered a slight setback. I'm uncertain of the best approach for grabbing track images for objects that have a track_id (e.g. iterations, submissions).

I almost considered making an API call in the track_image helper, but I think that might be too much for a view helper.

Instead, I'm thinking I'll create a helper for the routes that can be responsible for fetching a track's image url. The fun part will be associating the images with the appropriate objects before sending everything to the view (e.g. notifications route)

Wonder if it's worth caching track images? Not sure what sort of load Exercism is under, but I can imagine those API calls adding up when someone has a large amount of notifications.

@kytrinyx
Copy link
Member Author

I'm going to inline the whole x-api into exercism.io, so the HTTP calls will go away eventually.

In the meanwhile I think we should deliver the url for the images along with the rest of the track-level data, which I believe gets cached. It's accessed via ::X::Track.find(id).

@kytrinyx
Copy link
Member Author

Oh, I think that's what you did over in #2941 except that the x-api isn't delivering the images yet.

@joeltaylor
Copy link

Ah, ok - I saw that method and assumed it was a separate API call. It'll work perfectly! Inlining x-api sounds interesting, excited to see what that looks like.

I've been serving the images locally with x-api. I'll go ahead and submit a PR for that 😄

Thanks!

@joeltaylor
Copy link

joeltaylor commented Jun 13, 2016

Provide the URL to the track icon in the basic track data that x-api serves up

I've created this endpoint in a PR - x.exercism.io/v3/tracks/:track_id/img/icon? It allows for a track to add the icon filetype to their config.json. Should that URL still be provided or assumed the endpoint?

@kytrinyx
Copy link
Member Author

Let's provide the endpoint rather than assuming it—that way we can change our mind about where we serve it from without the web app having to know about the change.

@joeltaylor
Copy link

That makes sense. The moment I typed the word "assumption" I knew something wasn't right there. Thanks for the feedback! Should be able to get to these changes tonight.

@kytrinyx
Copy link
Member Author

Should be able to get to these changes tonight.

Excellent. I'm writing up that blazon issue now.

@petemcfarlane
Copy link

I tried to submit a PR for the xphp track but came up with this error in Travis:

$ ./bin/configlet .
Evaluating .
-> config.json does not include [img].
The command "./bin/configlet ." exited with 1.

Does anyone know how to solve this?

Thanks

@kytrinyx
Copy link
Member Author

Yeah, stick "img" in the "ignored" section in the config.json.

@petemcfarlane
Copy link

eureka!

@kytrinyx
Copy link
Member Author

Aaaaand: @ryanplusplus is updating configlet to ignore the img directory by default, so once that's done we won't have to stick it in the config.

martinsvalin added a commit to martinsvalin/xruby that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xracket that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xrust that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xscala that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xscheme that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xswift that referenced this issue Jun 16, 2016
martinsvalin added a commit to martinsvalin/xvbnet that referenced this issue Jun 16, 2016
@martinsvalin
Copy link

I noticed the list of opened issues stopped at exercism/xr, so I figured whatever script created them aborted at that point. I've opened PRs for the languages that the script missed.

@kytrinyx
Copy link
Member Author

Dammit. I bet I got rate limited and my script didn't report it. Thank you!

@kytrinyx
Copy link
Member Author

This has been done.

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

No branches or pull requests

4 participants