Skip to content

two-fer, hexadecimal, nucleotide-codons: Removed the deprecated exercises#773

Merged
coriolinus merged 2 commits intoexercism:masterfrom
ZapAnton:remove_deprecated_exercises
May 6, 2019
Merged

two-fer, hexadecimal, nucleotide-codons: Removed the deprecated exercises#773
coriolinus merged 2 commits intoexercism:masterfrom
ZapAnton:remove_deprecated_exercises

Conversation

@ZapAnton
Copy link
Copy Markdown
Contributor

As per this comment

Copy link
Copy Markdown
Member

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!

Looking at check-exercises.sh, it appears that that we didn't anticipate ever deleting exercises. As such, I don't expect Travis to pass. A manual check of this branch with the command below completed successfully.

for ex in exercises/*; do
   (
     cd "$ex"
     ../../bin/test-exercise
   )
done

@coriolinus
Copy link
Copy Markdown
Member

I plan to merge on Wednesday if nobody objects. I do not object if other maintainers merge this sooner.

@petertseng
Copy link
Copy Markdown
Member

Wow, configlet lint might still expect even deprecated exercises to exist as long as they are listed in config.json. Now, note that for deprecated exercises:

  • don't show up anywhere on the website if I have not already completed them
  • do show up if I already completed them
  • can still be fetched by explicitly specifying their ID

Since removing their implementations would cause configlet lint to fail, perhaps they will also have to be removed from there as well. I can't predict the effects of this

@coriolinus
Copy link
Copy Markdown
Member

Travis fails due to configlet lint:

0.28s$ ./bin/configlet lint .
-> An exercise with slug 'nucleotide-codons' is referenced in config.json, but no implementation was found.
-> An exercise with slug 'two-fer' is referenced in config.json, but no implementation was found.
-> An exercise with slug 'hexadecimal' is referenced in config.json, but no implementation was found.
The command "./bin/configlet lint ." exited with 1.

While we could fix this by removing the deprecated exercises from config.json, I believe there to be value in retaining the deprecated exercises in config.json: this lets the maintainers see quickly in the future that these exercises used to be in the track, and were removed. I have filed a configlet issue requesting lint changes for this case.

I will therefore override Travis and merge this (finally!) anyway.

@coriolinus coriolinus merged commit 168932a into exercism:master May 6, 2019
@ZapAnton ZapAnton deleted the remove_deprecated_exercises branch May 6, 2019 11:51
coriolinus added a commit to coriolinus/exercism_rust that referenced this pull request May 8, 2019
The rust track had several deprecated exercises. As they were removed
from the list of exercises available to students some time ago,
exercism#773 removed the actual exercises,
in order to clean things up. This produced a CI failure, but it was
overridden, because it was a configlet bug, not a real problem.

Overriding the CI failure was a mistake: I hadn't considered that
this would cause _every_ CI job to fail from that point forward.

I opened exercism/configlet#160 fixing the
issue with configlet, but it has not yet been reviewed, and there
will be an unknowable amount of time before it is reviewed and
approved. In order to make configlet stop complaining and restore
CI functionality to the track, this PR moves the removed exercises
into the foregone list, which doesn't suffer the configlet bug.
@petertseng
Copy link
Copy Markdown
Member

Regarding whether it is safe to remove the code for deprecated exercises: My read is that it is safe. I show my work:

One may use the search "add back deleted, deprecated exercises"[1] to see the following quote
[1] https://github.com/search?q=org%3Aexercism+%22Add+back+deleted%2C+deprecated+exercises%22&type=Issues

When migrating the data from v1 to v2, we will lose all the solutions submitted to deprecated exercises, unless they exist in the track configuration.

We can delete them from the track again after we launch v2, if we want to.

(emphasis mine)

Note that git log -p -S deprecated shows that the most recent deprecation is June 2018, so only students who have been around for a long time will have seen the deprecated exercises. This means impact should be small.

The only value in potentially doing so is if a student can choose to still download it and complete anyway, but we have better alternatives for them:

  • nucleotide-codons -> protein-translation
  • hexadecimal -> all-your-base
  • two-fer -> any exercise that actually does Option properly

@coriolinus
Copy link
Copy Markdown
Member

Per new documentation in exercism/docs#126, this was not actually a safe change. I'd also believed that it was safe, but it's proven to be a problem. I will revert this.

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

Successfully merging this pull request may close these issues.

3 participants