exercism / problem-specifications

Shared metadata for exercism exercises.
MIT License
326 stars 541 forks source link

Fix inconsistent 'factor of x' description in raindrops #2330

Closed BNAndras closed 11 months ago

BNAndras commented 11 months ago

All other descriptions referencing factor say 'factor x', but this test's description says 'factor of x' .

BNAndras commented 11 months ago

I'm fine leaving it alone then.

ErikSchierboom commented 11 months ago

This should mostly not be visible to the student and changing this can cause churn in downstream track test files. Is there value in changing this? Does that value outweigh the consequences (making generated tests go out of sync)?

I think the value is just consistency, and I'm totally fine with that. The amount of churn this generates is not much and we also don't requeue solutions when they are upgraded but only the tests file has changed, so there is little downside I feel.

BethanyG commented 11 months ago

I think the value is just consistency, and I'm totally fine with that. The amount of churn this generates is not much and we also don't requeue solutions when they are upgraded but only the tests file has changed, so there is little downside I feel.

I am a bit confused here, because when a test file/test case changes, how do we know that the solution still passes? I would assume that that is the exact scenario when re-queueing would happen. Is that because the GUID stays the same, so the file is not technically "different" enough? Or is it a change that will need to be flagged at the track level to prevent re-queuing?

For the Python test generator, a change in description means a change to the name of the test in the test file, and I would assume a re-queue unless flagged. For this exercise, that's 17, 133 solutions. Fine to do that -- but it does seem a bit much considering the change is the removal of the word of.

ErikSchierboom commented 11 months ago

Doh, I somehow misread 🤦 It does mean requeuing when the tests file change, unless the magic marker is present. Let's close this after all. My bad all!