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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃 Add stub files to all exercises #478

Merged
merged 1 commit into from
May 20, 2021

Conversation

ErikSchierboom
Copy link
Member

In this PR, we created (empty) stub files for all exercises that didn't yet have them.

The lack of stub file generates an unnecessary pain point within Exercism, contributing a significant proportion of support requests, making things more complex for our students, and hindering our ability to automatically run test-suites and provide automated analysis of solutions.

The original discussion for this is at exercism/discussions#238.

We will automatically merge this PR in two week's time if it has not been merged by track maintainers at that point 馃檪

Tracking

exercism/v3-launch#33

@ErikSchierboom ErikSchierboom added the v3-migration 馃 Preparing for Exercism v3 label May 4, 2021
@iHiD iHiD merged commit 638c47d into exercism:main May 20, 2021
@gypsydave5
Copy link
Contributor

gypsydave5 commented Sep 9, 2021

If this commit was reverted then the current issues with fetching exercises in the Pharo image would be solved. In addition I'm not entirely sure the problems outlined in exercism/discussions#238 apply to Pharo, as there are no files that the student will ever see. The presence of the stub files in the exercises means that classes would have to be added to them, which seems to be a bit more than is intended by this PR.

Any opinions, objections, thoughts?

@ghost
Copy link

ghost commented Sep 10, 2021

My oponion is to revert this PR because when you use the TDD way , so press on a test and then use the debugger to make the classes and files a user nevers had to learn how to make a class or a function.

Right now we cannot make Pharo work because of these stub files

@ErikSchierboom @iHiD

@Bajger
Copy link
Contributor

Bajger commented Sep 10, 2021

Following comment is related to issue: #499 (comment)

Hi all!
Just to explain in wider context:
Pharo dev environment (running image by VM) loads all .st definitions from repository into object memory image and then it is shown to user in system browser (kinda editor).

Only benefit for beginner users might be that they will see prepared empty class with exercise name e.g.: Grains or TwoFer, where they would be able to define new methods.

On the other hand, part of learning experience is also to create new class. Learning curve is following:

  1. Beginner user tries to run tests by clicking a button on test class.
  2. Debugger is opened with a runtime error that class definition is expected in tested code.
  3. Directly from debugger they can lively create new class with desired name (IDE alredy provides stub code for a new class) and hits "continue" in debugger with program flow (until some other error occurs).
  4. Rationale behind this that it truly shows TDD process, where you write a test first, run a test and immediatelly implement code of program as runtime of test evolves.

@Bajger
Copy link
Contributor

Bajger commented Sep 10, 2021

Decision is up to you: If you still prefer consistency across all tracks and want to have a stub file, we can provide empty class definition into those files. Empty file for Pharo track doesn't really provide too much value for beginners, since they aren't forced to create any file. Dev environment (running image) keeps all code in object memory and uses files just for exporting purpose to Exercism server (submit action creates .st files from object memory and pushes them to server).

@ErikSchierboom
Copy link
Member Author

It is hard for me to properly gauge the impact of stub files, so I'll defer to your judgment. In principle, I'd be fine with making an exception for Pharo as it looks like otherwise things don't work and the proper Pharo TDD workflow is different, but there are some issues:

  1. configlet will complain about a missing stub file, leading to broken CI. We could make an exception for Pharo, but it would require a bit of work.
  2. If a test runner were to be implemented, the online editor would be enabled but in a broken state as it needs a stub file to be shown. We'd have to do work to enable this scenario.

In both cases, it would require us to implement things.

On the other hand, part of learning experience is also to create new class.

Well yes, but do students really learn much from having to create a new class for the 30th exercise they're doing? I would argue that at that point it becomes repetitive and doesn't provide the student much in the way of learning. 馃し

CC @iHiD

@ghost
Copy link

ghost commented Sep 10, 2021

@ErikSchierboom no, when you follow the flow Bagjer has described , user do NOT have to create a class at all.
The Pharo debugger is smart enough to make a class for you. It can also make the functions/method body for you if asked

@Bajger
Copy link
Contributor

Bajger commented Sep 10, 2021

Hi Erik,
If removing stub file causes implementation effort on Exercism side, it is easier to adopt on Pharo track side. Basically we could do:
a) Put empty class for each exercise stub file.
b) Treat it on Pharo code and test nil condition. Empty file would be simply ignored, so that users will be responsible for creating dedicated class solution. By submission action, class will be exported in .class..st file and submitted to server.
To your last comment: I agree with you, there isn't too much value of doing same "create class" action from debugger for each exercise again and again. I also believe, that learning how to create a new class could be part of concept exercise, where more explanation can be served to new users of Pharo track.

So in general: We can provide class definitions (option a) is preferred one for me) and submit PR. It would save effort for Exercism maintainers for something more useful :-)

PS: one last question - Is stub file also required also for concept exercises? There is slight deficiency that concept exercise might not be code relevant in context of solution class. (Basically if you want to learn 1 + 2 in Pharo, you don't need to create class and method for it, you just evaluate it as REPL-like command.

Let me know your thoughts!

@ErikSchierboom
Copy link
Member Author

@Bajger Option a) would work well for us!

It would save effort for Exercism maintainers for something more useful :-)

Much appreciated! We're already quite loaded with work :)

Is stub file also required also for concept exercises?

They are.

@ErikSchierboom
Copy link
Member Author

Basically if you want to learn 1 + 2 in Pharo, you don't need to create class and method for it, you just evaluate it as REPL-like command.

Other tracks that have a REPL don't use that to teach about the language, but instead use methods (and classes if needed), as there is no way to easily verify what the student has done when the student uses a REPL (at least for other tracks).

@Bajger
Copy link
Contributor

Bajger commented Sep 10, 2021

I see, in Pharo there is possibility to serialize REPL-like code from something called "Playground" (simple REPL-like editor where you evaluate) into an .ph file - basically it is just string contaning Pharo code. Such string can be evaluated by image during its startup (from what I understood, it is responsibility of Docker image containing Pharo dev environment):
$ ./pharo Pharo.image eval "1+2" (we could pass a file as argument as well)

So can be stub file freely named? (e.g. <exercise-slug>.ph) or it ha to follow some convention?

@ErikSchierboom
Copy link
Member Author

The stub can be freely named.

@iHiD
Copy link
Member

iHiD commented Sep 10, 2021

If changing stub names, ensure that it is set here: https://github.com/exercism/pharo-smalltalk/blob/main/exercises/practice/allergies/.meta/config.json#L11

Thanks for sorting this @Bajger (and @ErikSchierboom facilitating)

@ErikSchierboom ErikSchierboom added the x:size/tiny Tiny amount of work label Oct 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3-migration 馃 Preparing for Exercism v3 x:size/tiny Tiny amount of work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants