-
Notifications
You must be signed in to change notification settings - Fork 35
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
What is the point of solution dir & translated solutions? #19
Comments
Thank you for taking the time to write this up. Actually I am available at gitter that should have cleared a lot of questions. There are two reasons for the current structure:
|
@martinheidegger Thx for reply. What problem does this solve? Can you point to any other project that maintains multiple copies of its codebase, one per language? |
This decision is up to @rvagg & whoever else ultimately so I'll leave it alone, but I want to make clear that I'm not saying this feature is a superfluous nice-to-have, I'm saying this feature is a bug. A feature that encourages users to...
is a bad feature & damages the client code written on the framework. The only rationale for this feature I've seen so far is what you write above:
If in fact it was important lesson content, writing it into the solution file was a bad choice. The framework should not be rewritten to accommodate mistakes like this, especially when it impacts many other people's packages. For example: this is just the English version with different formatting. I assume it's obvious why this is a undesirable. Translators are now reorganizing existing lessons to propagate this mistake into them. I hope you'll consider the maintenance impact this will have on workshoppers as well as the difficulty encouraging people to hide lesson hints deep in the workshop code will cause for learners! And the fact that there's already an i18n tool to handle string translation. Anyway I'll get back to my nodeschool lesson now, do what you want with this issue. 😸 |
Working on my lesson-set I see that the solution is printed after a lesson is successfully completed. Is this the place where translation is desired? This makes sense but I still think the maintenance tradeoff is not worth it-- better include a |
Language specific solution files are discouraged! It is there because the solution will be showed to the user and thus contains "information" to be shown to the user (as you just figured out) |
I'm creating a new workshopper on Workshopper 2.3.1. In creating my first exercise I came across the
solution
dir. I'm usinglearnyounode
for examples so I checked what was in these directories.It didn't seem to be that there was much in these dirs besides a solution.js.
I checked to find out what was going on & saw that it's for translated solution files. This confused me because I wondered what would need translating in a solution file. I found some translated solutions in
my_first_io
& took a look:==> solution/solution.js <==
==> solution_fr/solution.js <==
==> solution_ja/solution.js <==
The only thing translated seems to be... the comments? 😕
I looked further to see if I could find the exercise that needed to have translated versions of the solution.
==> solution/solution.js <==
==> solution_fr/solution.js <==
==> solution_ja/solution.js <==
❓ ❔ ❓ There's an i18n framework in place for this.
My Point
I believe that the "translated solutions" is a 👎 bad idea. Here's why:
en
solution file, do you hold the PR & tell them to go update all 7 or however many translation files?en
solution is patched & the others are overlooked because maintainer only speaksen
? Now the solution behavior has diverged by language.lang.json
&problem_lang.md
, the translators need to comb thru every exercise looking for untranslated/updated comments? 😦solution.js
?solution.js
is not the place to put exercise hints! How would the average nodeschooler even know to look here? If// note you can avoid the .toString() by passing 'utf8' as the second argument to readFileSync, then you'll get a String!
is important for people to read, put it in theproblem.md
where people can find it.❓ What problem is this feature solving?
Proposal
Load
exercises/my_foo/solution.js
& call it a day. I will try to submit a PR given the green light.Why I even care about this
Because I'm authoring a workshopper & it's bothersome to me, but more importantly, stuff like this needlessly makes it more difficult & complex to create a workshopper! The new workshopper & workshopper-excercise APIs have introduced a lot of complexity & I am arguing for the potential author who just wants to make a new workshopper easily.
The text was updated successfully, but these errors were encountered: