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

Add missing deprecation notices #2974

Merged
merged 3 commits into from
Mar 16, 2020
Merged

Add missing deprecation notices #2974

merged 3 commits into from
Mar 16, 2020

Conversation

yscik
Copy link
Contributor

@yscik yscik commented Mar 14, 2020

Some methods only had a @deprecated tag with no user-facing warning. This adds the appropriate _deprecated_function calls.

  • Sensei_Learner_Management::get_learner_full_name - replaced by Sensei_Learner::get_full_name
  • Sensei_Frontend::sensei_get_template_part - replaced by Sensei_Templates::get_part
  • Sensei_Frontend::sensei_course_image - replaced by Sensei()->course->course_image
  • Sensei_Frontend::sensei_lesson_image - replaced by Sensei()->lesson->lesson_image
  • Sensei_Frontend::sensei_lesson_quiz_meta - replaced by Sensei_Lesson::footer_quiz_call_to_action
  • Sensei_Utils::sensei_save_quiz_answers - replaced by Sensei_Quiz::save_user_answers
  • Sensei_Utils::sensei_grade_quiz_auto - replaced by Sensei_Grading::grade_quiz_auto
  • Sensei_Utils::sensei_grade_question_auto - replaced by Sensei_Grading::grade_question_auto
  • Sensei_Utils::sensei_get_user_question_answer_notes - replaced by Sensei()->quiz->get_user_question_feedback
  • quiz_questions - replaced by Sensei_Templates::get_template
  • sensei_check_prerequisite_course - replaced by Sensei_Course::is_prerequisite_complete

Testing instructions:

  • Using these methods should show a user error when WP_DEBUG is on.

@yscik yscik added the Deprecation This change introduces a deprecation. label Mar 14, 2020
@yscik yscik requested a review from a team March 14, 2020 00:32
Copy link
Contributor

@gikaragia gikaragia left a comment

Choose a reason for hiding this comment

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

I think that we should use the current version for deprecation otherwise we will remove these functions too early.

@renatho
Copy link
Contributor

renatho commented Mar 16, 2020

I agree with @gkaragia about the version. And the rest LGTM!

@yscik
Copy link
Contributor Author

yscik commented Mar 16, 2020

@jom found that this was the way it was handled previously, like in #2423.
I'd rather not add conflicting versions, so if we use 3.0.0, it would probably be best to update the @deprecated tags too.
We can also just add these to a Remove deprecated stuff for 5.0 ticket and track it that way.

gikaragia
gikaragia previously approved these changes Mar 16, 2020
Copy link
Contributor

@gikaragia gikaragia left a comment

Choose a reason for hiding this comment

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

The reason that I asked for this is to make sure that it takes 2 major version bumps before removing the methods. It's totally fine if we add the changes to the 5.0 ticket and make sure that we don't remove them earlier.

renatho
renatho previously approved these changes Mar 16, 2020
Copy link
Contributor

@renatho renatho left a comment

Choose a reason for hiding this comment

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

We can also just add these to a Remove deprecated stuff for 5.0 ticket and track it that way.

I think it makes sense! Just to prevent a mistake, maybe we could also add a comment like this: https://github.com/Automattic/sensei-wc-paid-courses/blob/master/includes/woocommerce-integrations/class-sensei-wc.php#L739

WDYT?

@yscik
Copy link
Contributor Author

yscik commented Mar 16, 2020

Yup that's a good idea!

@yscik yscik dismissed stale reviews from renatho and gikaragia via f4f8c23 March 16, 2020 15:04
@yscik yscik merged commit ea76088 into master Mar 16, 2020
@donnapep donnapep added this to the 3.0.0 milestone Mar 19, 2020
@donnapep donnapep deleted the add/deprecation-notices branch March 19, 2020 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation This change introduces a deprecation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants