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

Include code smell under refactoring #108

Closed
wants to merge 1 commit into from

Conversation

tlylt
Copy link

@tlylt tlylt commented May 27, 2021

Code smell might be a worth knowing concept when learning about
refactoring.

Let's include it under refactoring so that learners can be aware of the
term and pick up some ideas on how code smells might indicate a
refactoring is overdue.

I have also added a list of links for code smell under reference.

Fixes #107

P.S Hi prof, not sure why the deployment failed, if there is any required changes on my end please feel free to let me know. Thank you.

Code smell might be a worth knowing concept when learning about
refactoring.

Let's include it under refactoring so that learners can be aware of the
term and pick up some ideas on how code smells might indicate a
refactoring is overdue.

I have also added a list of code smell links under reference.
@damithc
Copy link
Contributor

damithc commented May 28, 2021

Thanks for the PR @tlylt Looks like you put in a good amount of effort into this 💯
Given this is textbook, it is not easy for external contributors to add content (beyond smaller bug fixes) -- as there are many different aspects to balance. For example, the depth of the topic should match the other topics, and it should not introduce unnecessary dependencies between topics (as different courses uses different orderings of the topics). I will factor in your proposed content when I add this topic to the book (hopefully, later in the summer). Most likely it will be a shorter version of your proposed contents. I'll leave this PR in view until then.

@tlylt
Copy link
Author

tlylt commented May 28, 2021

Thanks for the PR @tlylt Looks like you put in a good amount of effort into this 💯
Given this is textbook, it is not easy for external contributors to add content (beyond smaller bug fixes) -- as there are many different aspects to balance. For example, the depth of the topic should match the other topics, and it should not introduce unnecessary dependencies between topics (as different courses uses different orderings of the topics). I will factor in your proposed content when I add this topic to the book (hopefully, later in the summer). Most likely it will be a shorter version of your proposed contents. I'll leave this PR in view until then.

Sure thing Prof, thank you for your kind reply!

damithc pushed a commit that referenced this pull request Aug 4, 2021
@damithc
Copy link
Contributor

damithc commented Aug 4, 2021

I've added a light mention of code smells in edca446, and mentioned this PR in the commit message. Adding more detailed version can be considered in a future date. Thanks again for your work in this PR @tlylt

@damithc damithc closed this Aug 4, 2021
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.

Contribution guideline?
2 participants