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 unreachable code #2947

Merged
merged 15 commits into from Jun 22, 2016

Conversation

Projects
None yet
4 participants
@joshuaclayton
Contributor

joshuaclayton commented Jun 17, 2016

Hi all!

I've been working on a tool to identify unused code recently, and while I was listening to the episode with @kytrinyx, she'd mentioned refactoring and needing help on the app, so, here I am!

This is a fairly large set of commits, but each discrete commit outlines removal of unused code, as well as the corresponding commit(s) that led up to it becoming unreachable.

I've attempted to make each commit both as discrete and thorough as makes sense (basically, I didn't want my removal to introduce further methods needing to be removed in other commits, creating a dependency).

Two commits include removal of Rubocop configuration - disabling long classes - that I included to keep the test suite green.

I've not been able to get lineman running locally, for whatever reason, so I'd only been running the test suite and Rubocop throughout this process.

I verified this in a browser, although there are others who might know where to be more thorough.

Please let me know if there's anything else I can do to help or explain these changes; I realize it's a fair bit to go through!

joshuaclayton added some commits Jun 17, 2016

Remove unreachable instance methods on ExercismWeb::Presenters::Profile
This removes the instance methods track_ids and archived_in from the
profile presenter; these methods moved into an unreachable state in the
commit 38ce25d.
Remove ExercismWeb::Presenters::Sharing
This removes the twitter_link instance method (and subsequently, the
entire ExercismWeb::Presenters::Sharing class), whose usage was removed
in e3648f0 and whose test was removed in
82d117d.
Remove unreachable code from ExercismWeb::Helpers::FuzzyTime
This removes the following instance methods:

* days
* weeks
* months

These methods' usage were removed in
acbd567.
Remove LanguageTrack and corresponding code
This removes the LanguageTrack class, whose usage was removed in
c0cba48 and
5aaf4ba.

This also removes related test harness work, including a fixture file
and helper method.
Remove ExercismWeb::Presenters::Assignment and corresponding files
This removes the assignment presenter, corresponding test, and fixture
file only used by the test. This presenter stopped being used in
d3cf318.
Remove UnknownLocale and UnknownLanguage
This removes two unused classes related to locale and location lookup.
UnknownLocale was removed in b52ea35,
while UnknownLanguage's final usage is removed in this commit;
removals from other areas of the codebase occurred in
6ab1b69 and
cd4eaca.
Remove unused test helper methods and corresponding fixture
This removes AppPresentersHelper, which contained two methods, both of
which were unused.

ruby_track_problems_array was introduced in
671309c but was never used in
subsequent commits; usage of all_problems_json was removed in
48962e2.
Remove two unused query objects
This removes SubmissionsSQL and NitpicksSQL classes, as their usage was
removed in ae9251b.
Remove Guest#onboarding_steps
This removes an unused instance method onboarding_steps on the Guest
null object. The corresponding method on User was removed in
634f751.
Remove unused older_than access on Submission
This removes the Submission#older_than?, as well as the scope
Submission.older_than, which were removed in
97ca051 and
c775850, respectively.

Additionally, this removes a RuboCop setting on the same Submission
file, as RuboCop was failing due to unnecessary disabling of
Metrics/ClassLength.
Remove active_tracks helper within ExercismWeb::Routes::Core
This removes the active_tracks helper, as well as the corresponding
ExercismWeb::Presenters::Languages class (now unused), and corresponding
test stubs that were also unnecessary.

The most recent change removing usage of active_tracks was in
ab545eb.
Remove unused exception InvalidAttemptError
This removes the unused exception InvalidAttemptError, whose usage was
removed in fe314dd.
Remove dashboard_assignment_nav and dashboard_assignment_section_nav
Both helper methods within ExercismWeb::Routes::Core were removed with a
restructure of the views, where the markup was removed in
c775850.
@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Jun 17, 2016

Coverage Status

Coverage increased (+0.1%) to 80.287% when pulling fbc8e20 on joshuaclayton:remove-unreachable-code into 7c2ffae on exercism:master.

coveralls commented Jun 17, 2016

Coverage Status

Coverage increased (+0.1%) to 80.287% when pulling fbc8e20 on joshuaclayton:remove-unreachable-code into 7c2ffae on exercism:master.

@kytrinyx

This comment has been minimized.

Show comment
Hide comment
@kytrinyx

kytrinyx Jun 21, 2016

Member

Oh my goodness this is brilliant! ❤️ ❤️ ❤️ (I literally have tears in my eyes because this makes me so happy and grateful).

I will be going through this carefully over the next few days!

Member

kytrinyx commented Jun 21, 2016

Oh my goodness this is brilliant! ❤️ ❤️ ❤️ (I literally have tears in my eyes because this makes me so happy and grateful).

I will be going through this carefully over the next few days!

@sharang-d

This comment has been minimized.

Show comment
Hide comment
@sharang-d

sharang-d Jun 21, 2016

1500 lines removed! Good job 👍

1500 lines removed! Good job 👍

@kytrinyx

This comment has been minimized.

Show comment
Hide comment
@kytrinyx

kytrinyx Jun 22, 2016

Member

This is so good. Thank you again for taking the time to find all of this and rip it out! ❤️

Member

kytrinyx commented Jun 22, 2016

This is so good. Thank you again for taking the time to find all of this and rip it out! ❤️

@kytrinyx kytrinyx merged commit fd8cec2 into exercism:master Jun 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment