Skip to content

Conversation

@StlMaris123
Copy link
Collaborator

@StlMaris123 StlMaris123 commented Oct 3, 2019

In your PR did you:

  • Create CaseMailer
  • Solves Create CaseMailer #3200
    • Include a description of the changes?
    • Mention the issue the PR addresses?
    • Include screenshots of any changes to the UI?
    • Isolate any changes to gems (meaning that any new, updated, or removed gems and resulting code changes should be in their own PR)?
    • Add and/or update specs for your code?

@StlMaris123 StlMaris123 requested a review from rlgreen91 October 3, 2019 14:32
@trystant
Copy link
Contributor

trystant commented Oct 4, 2019

@StlMaris123 have you run rubocop -a on the files in this PR?

@StlMaris123
Copy link
Collaborator Author

Hi @trystant , yes I did...It is a refactoring issue on the case mailer

@rlgreen91
Copy link
Contributor

Heyo, so I would like to hold this PR - someone else is working on renaming this mailer from UserNotifier to UserMailer, and I'd like to merge his PR first.

@rlgreen91
Copy link
Contributor

@StlMaris123 can you resolve these merge conflicts? Then I'll be able to merge this.

…iler

# Conflicts:
#	app/controllers/cases_controller.rb
#	app/mailers/user_notifier.rb
#	app/views/case_mailer/send_deletion_email.html.erb
#	app/views/case_mailer/send_followers_email.html.erb
#	app/views/user_mailer/send_deletion_email.html.erb
#	app/views/user_mailer/send_followers_email.html.erb
#	app/views/user_notifier/send_deletion_email.html.erb
#	app/views/user_notifier/send_followers_email.html.erb
#	spec/mailers/previews/user_notifier_preview.rb
#	spec/mailers/user_mailer_spec.rb
@@ -0,0 +1,28 @@
# frozen_string_literal: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use keyword arguments.

@rlgreen91
Copy link
Contributor

Hey @StlMaris123 are you going to be able to finish this soon?

@rlgreen91
Copy link
Contributor

@StlMaris123 one last thing - can you take care of the CodeClimate failures? You can ignore the ones about similar blocks of code, but fix the line too long and missing top-level documentation one.

Copy link
Contributor

@rlgreen91 rlgreen91 left a comment

Choose a reason for hiding this comment

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

LGTM

@rlgreen91 rlgreen91 merged commit c2f899f into master Nov 1, 2019
@rlgreen91 rlgreen91 deleted the create_case_mailer branch November 1, 2019 00:57
@rlgreen91 rlgreen91 mentioned this pull request Nov 1, 2019
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.

4 participants