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.

Sign up for GitHub

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

Jump to bottom

Simplify UserScript#empty?. #14842

Merged
merged 1 commit into from May 4, 2017
Merged

Simplify UserScript#empty?. #14842

merged 1 commit into from May 4, 2017

Conversation

ashercodeorg
Copy link
Contributor

@ashercodeorg ashercodeorg commented May 4, 2017 •
edited

Not only is it impossible in theory, it is impossible in practice.

SELECT COUNT(0)
FROM user_scripts
WHERE started_at IS NULL AND (completed_at IS NOT NULL OR last_progress_at IS NOT NULL);
+----------+
| COUNT(0) |
+----------+
|        0 |
+----------+

ashercodeorg
ashercodeorg commented May 4, 2017
View reviewed changes
dashboard/app/models/user_script.rb
@@ -33,7 +33,6 @@ def check_completed?
end

def empty?
# a user script is empty if there is no progress
started_at.nil? && completed_at.nil? && assigned_at.nil? && last_progress_at.nil?
started_at.nil? && assigned_at.nil?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aside, I'm not certain where or why this method is being used.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to delete this if it's completely unused, but that's not terribly urgent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Unfortunately, our codebase uses empty? in other contexts quite a bit.

@ashercodeorg ashercodeorg merged commit 5b4c731 into staging May 4, 2017
@ashercodeorg ashercodeorg deleted the simplifyUserScriptEmpty branch May 4, 2017 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@balderdash balderdash balderdash approved these changes

Assignees
No one assigned
Labels
code-hygiene
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ashercodeorg @balderdash