Skip to content

Revert removing deprecated exercises#832

Merged
coriolinus merged 4 commits intoexercism:masterfrom
coriolinus:revert-remove-deprecated
May 11, 2019
Merged

Revert removing deprecated exercises#832
coriolinus merged 4 commits intoexercism:masterfrom
coriolinus:revert-remove-deprecated

Conversation

@coriolinus
Copy link
Copy Markdown
Member

Per new documentation in exercism/docs#126, it is actually not safe to remove deprecated exercises. As such, #773 should not have been approved. Without #773, we don't need #830.

This reverts both of those PRs.

- regenerate readmes
- copy examples to lib.rs so they compile
Copy link
Copy Markdown
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

have a pre-emptive approval for when you are satisfied with what Travis CI tells you, and potentially removing the given implementations. feel free to ask for another review if you want it though.

I will put forth the option of excluding deprecated exercises from the README up-to-date check, but I note the following:

  • Anyone who uses configlet generate to generate READMEs would still cause README updates on the deprecated exercises
  • Increased complexity in the scripts

I think I do not support my own suggestion, but I welcome it if others can find a way to make it work well.

@@ -1 +1,33 @@
fn parse_hex_digit(c: char) -> Option<i64> {
match c {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

although it does not hurt to have it since it's deprecated, this lib.rs doesn't necessarily need to have a complete implementation (and we would want it not to have one if it were non-deprecated)


impl<'a> CodonInfo<'a> {
pub fn name_for(&self, codon: &str) -> Result<&'a str, &'static str> {
if codon.len() != 3 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

although it does not hurt to have it since it's deprecated, this lib.rs doesn't necessarily need to have a complete implementation (and we would want it not to have one if it were non-deprecated)

same

@@ -1,3 +1,6 @@
pub fn twofer(name: &str) -> String {
unimplemented!("One for {}, one for me.", name);
match name {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

although it does not hurt to have it since it's deprecated, this lib.rs doesn't necessarily need to have a complete implementation (and we would want it not to have one if it were non-deprecated)

same

@coriolinus
Copy link
Copy Markdown
Member Author

I copied the full implementations over the three stub src/lib.rs implementations because there were some compilation errors, and I didn't believe it was worth the effort of figuring out and fixing the real problems there.

As you've already approved this (thank you!), and Travis now passes, I will now merge this.

@coriolinus coriolinus merged commit 9fd8d82 into exercism:master May 11, 2019
@coriolinus coriolinus deleted the revert-remove-deprecated branch May 11, 2019 18:27
@petertseng
Copy link
Copy Markdown
Member

petertseng commented May 13, 2019

Given that the documentation claims that students who have not already started a deprecated exercise will not be able to do so, it is OK to have stubs that already provide a solution.

For the purposes of our track upholding its documented obligations, it is irrelevant that the aforementioned claim is false (I can currently download hexadecimal and submit an implementation to it, not having started it previously); that issue must be solved elsewhere, so this track need not take specific action on that.

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.

2 participants