Skip to content

Conversation

@vaeng
Copy link
Contributor

@vaeng vaeng commented Mar 2, 2023

The #12in23 challenge asks to do the linked-list exercise, which is currently not implemented and should be added during the month of March. As a preliminary task, we include a simpler version that can be built upon.

  • Basic file system
  • Write tests
  • Write exercise stub
  • Write an example solution
  • Write optional documentation files

@vaeng vaeng requested a review from siebenschlaefer March 2, 2023 14:12
@vaeng vaeng self-assigned this Mar 2, 2023
@vaeng
Copy link
Contributor Author

vaeng commented Mar 2, 2023

I wonder how we can test that the student made sure that no resources have leaked.

@vaeng vaeng marked this pull request as ready for review March 3, 2023 11:14
@vaeng
Copy link
Contributor Author

vaeng commented Mar 3, 2023

I would like to merge this soon, so the users can get their March badges. Would you have a look @siebenschlaefer or @exercism/maintainers-admin

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Thanks a ton for adding this! I'll leave the C++ review to @siebenschlaefer.

@vaeng vaeng force-pushed the wip/Add-simple-linked-list branch from 2ea7f5a to d021fa0 Compare March 3, 2023 12:57
@ErikSchierboom
Copy link
Member

I'd like to get this PR merged sooner than later, as this is one of the recommended exercises for Mechanical March

@siebenschlaefer do you have time to do a review of this PR?
@vaeng could you look into fixing the diff for the config.json file to not include any formatting changes?

@vaeng
Copy link
Contributor Author

vaeng commented Mar 7, 2023

I'd like to get this PR merged sooner than later, as this is one of the recommended exercises for Mechanical March

@vaeng could you look into fixing the diff for the config.json file to not include any formatting changes?

I had to change the end of line sequence. I uploaded with CRLF but it was LF.

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

LGTM

@vaeng vaeng merged commit 6d6a80c into exercism:main Mar 7, 2023
@vaeng vaeng deleted the wip/Add-simple-linked-list branch March 7, 2023 14:20
@siebenschlaefer
Copy link
Contributor

siebenschlaefer commented Apr 2, 2023

I'm late to the party, sorry. I like this translation, good job!

I have one tiny suggestion: Use std::size_t instead of size_t. The latter will work on all major compilers but technically that's implementation-specific. After including <cstddef> the standard only guarantees the existence of std::size_t.

@vaeng vaeng restored the wip/Add-simple-linked-list branch April 2, 2023 16:40
@vaeng vaeng deleted the wip/Add-simple-linked-list branch July 5, 2023 08:19
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