Skip to content

ensure-stubs-compile: Deny all warnings except new_without_default#1030

Merged
petertseng merged 1 commit intoexercism:masterfrom
petertseng:clippy-stub-deny
Nov 20, 2020
Merged

ensure-stubs-compile: Deny all warnings except new_without_default#1030
petertseng merged 1 commit intoexercism:masterfrom
petertseng:clippy-stub-deny

Conversation

@petertseng
Copy link
Copy Markdown
Member

We don't find it too useful to have students implement Default.
All other warnings in stubs have been dealt with, and we find it useful
to ensure there are no further ones.

I've determined via https://github.com/petertseng/exercism-rust/actions/runs/373860276 that when #1027, #1028, #1029, and this are merged, there are no more Clippy warnings or errors in stubs.

Closes #659
This is appropriate because:

  • stubs and tests are now ensured to have no Clippy warnings or errors
  • examples are ensured to have no errors, and we're unconcerned about
    them having warnings since students don't see them
  • rustfmt is enforced

We don't find it too useful to have students implement Default.
All other warnings in stubs have been dealt with, and we find it useful
to ensure there are no further ones.

Closes #659
This is appropriate because:

* stubs and tests are now ensured to have no Clippy warnings or errors
* examples are ensured to have no errors, and we're unconcerned about
  them having warnings since students don't see them
* rustfmt is enforced
@petertseng
Copy link
Copy Markdown
Member Author

We don't find it too useful to have students implement Default.

Well, we should discuss that, since by "we" right now I just mean "I".

@coriolinus
Copy link
Copy Markdown
Member

We don't find it too useful to have students implement Default.

So, I'm a little conflicted about this, because clippy's recommendation is in fact good practice. In general, it's nice to be able to grab a T::default() without having to care too much about what that really means. On the other hand, in the context of these exercises, students aren't usually actually creating the types they define; the test suite does that. Making them add #[derive(Default)] to their structs is just busywork.

I am therefore convinced that it is not very useful to have students implement Default.

@petertseng petertseng merged commit 6fbf6e2 into exercism:master Nov 20, 2020
@petertseng petertseng deleted the clippy-stub-deny branch November 20, 2020 08:56
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.

Should rustfmt and clippy checks be added to the CI?

2 participants