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

Remove one Activity dependency from UserLevel. #13412

Merged
merged 3 commits into from Feb 24, 2017
Merged

Remove one Activity dependency from UserLevel. #13412

merged 3 commits into from Feb 24, 2017

Conversation

ashercodeorg
Copy link
Contributor

@ashercodeorg ashercodeorg commented Feb 23, 2017 •
edited

No description provided.

@ashercodeorg
Copy link
Contributor Author

Note that, happily, it seems as if UserLevelTest has tests covering this change.

@ashercodeorg ashercodeorg mentioned this pull request Feb 23, 2017
Closed
@ashercodeorg
Copy link
Contributor Author

CIrcleCI failures are legitimate, I need to transfer over the definitions of those constants (alternately use ActivityConstants). Hold off on reviewing until fixed...

Hamms
Hamms approved these changes Feb 24, 2017
View reviewed changes
dashboard/app/models/user_level.rb
end

def passing?
Activity.passing? best_result
return false if best_result.nil?
best_result >= ActivityConstants::MINIMUM_PASS_RESULT
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably these constants will at some point need to be renamed. Any reason for that point to not be now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I'm open to renaming, I don't see why doing so is imminent. Even without the activities table, I do think it may be helpful to continue having the notion of an Activity. For example, we might still want the model around for milestone posts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That said, if we were to rename, any suggestions as to what the new name should be?

@ashercodeorg ashercodeorg merged commit f91cc78 into staging Feb 24, 2017
@ashercodeorg ashercodeorg deleted the userLevelModelMethods branch February 24, 2017 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hamms Hamms Hamms approved these changes

Assignees

@Hamms Hamms

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ashercodeorg @Hamms