Skip to content

Replacing User#warned with User#warned?#15628

Merged
jeremyf merged 1 commit intomainfrom
jeremyf/refactor-warned-to-warned-question-mark
Dec 1, 2021
Merged

Replacing User#warned with User#warned?#15628
jeremyf merged 1 commit intomainfrom
jeremyf/refactor-warned-to-warned-question-mark

Conversation

@jeremyf
Copy link
Copy Markdown
Contributor

@jeremyf jeremyf commented Dec 1, 2021

What type of PR is this? (check all applicable)

  • Refactor

Description

A simply refactor to introduce more idiomatic method names.

Related Tickets & Documents

Related to #15624

QA Instructions, Screenshots, Recordings

None.

UI accessibility concerns?

None.

Added/updated tests?

  • No, and this is why: refactor of method names

[Forem core team only] How will this change be communicated?

  • This change does not need to be communicated, and this is why not: it's a simple refactor

A simply refactor to introduce more idiomatic method names.

Related to #15624
@jeremyf jeremyf requested a review from a team December 1, 2021 17:38
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Dec 1, 2021
@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Dec 1, 2021
Copy link
Copy Markdown
Contributor

@djuber djuber left a comment

Choose a reason for hiding this comment

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

Makes sense to me.

Copy link
Copy Markdown
Contributor

@juliannatetreault juliannatetreault left a comment

Choose a reason for hiding this comment

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

Left a quick question, but otherwise looks good!

@@ -62,7 +62,7 @@ def unsuspend_user

expect(user.suspended?).to eq(true)
expect(user.trusted).to eq(false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With all of these refactors, are we also planning on changing trusted to trusted?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe we already did (and it's just a change in the callers)?

https://github.com/forem/forem/blob/main/app/models/user.rb#L384

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed that, @djuber 👀

@jeremyf jeremyf merged commit e1bddb5 into main Dec 1, 2021
@jeremyf jeremyf deleted the jeremyf/refactor-warned-to-warned-question-mark branch December 1, 2021 18:14
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Dec 1, 2021
@djuber djuber mentioned this pull request Dec 8, 2021
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: merged bot applied label for PR's that are merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants