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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove duplicate not done comment #292

Merged
merged 2 commits into from
Apr 8, 2020
Merged

Conversation

Tuxified
Copy link
Contributor

@Tuxified Tuxified commented Apr 7, 2020

As indicated in #259 , I found it confusing to have 2 comments as the code wouldn't compile unless I solved both issues (I used the script from #281 to remove a comment and use :wn to go to next exercise, hence this tripped me).

As indicated in rust-lang#259 , I found it confusing to have 2 comments as the code wouldn't compile unless I solved both issues (I used the script from rust-lang#281 to remove a comment and use `:wn` to go to next exercise, hence this tripped me).
@sanjaykdragon
Copy link
Contributor

IMO the comment should be moved into the block above

@Tuxified
Copy link
Contributor Author

Tuxified commented Apr 8, 2020

I choose the bottom one 'cause it was closer to the code I was editing ( I used vim, hence easier to spot ), but I'll switch to the comment on top if that's the convention

@shadows-withal
Copy link
Member

@Tuxified that sounds good!

@Tuxified
Copy link
Contributor Author

Tuxified commented Apr 8, 2020

Ok, I made the switch to remove the bottom one. Any other remarks/questions ? Do you agree with this change ?

@shadows-withal shadows-withal merged commit 052f0aa into rust-lang:master Apr 8, 2020
pedantic79 pushed a commit to pedantic79/rustlings that referenced this pull request Apr 11, 2020
Remove duplicate not done comment
ppp3 pushed a commit to ppp3/rustlings that referenced this pull request May 23, 2022
Remove duplicate not done comment
dmoore04 pushed a commit to dmoore04/rustlings that referenced this pull request Sep 11, 2022
Remove duplicate not done comment
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