Skip to content

Conversation

rhymes
Copy link
Contributor

@rhymes rhymes commented Apr 11, 2019

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

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

where(published: true) is an omnipresent condition in selecting all kinds of articles in the code.
A dedicated scope makes it shorter to type and as self explanatory.

Some specifiers have been re-ordered to follow a similar structure of WHERE queries (I haven't been too methodical about it, the order in the end doesn't matter for Rails)

I've also disabled erblint's rubocop Layout/InitialIndentation because it raises a lot of false positives

Added to documentation?

  • docs.dev.to
  • readme
  • no documentation needed

@rhymes rhymes changed the title [WIP] Use published scope for articles Use published scope for articles Apr 11, 2019
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Apr 11, 2019
Copy link
Contributor

@benhalpern benhalpern left a comment

Choose a reason for hiding this comment

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

Nice overall improvement. Not only is it more concise, it allows us to redefine what "published" means in the future in a DRY way if we ever need to.

@pr-triage pr-triage bot added PR: reviewed-approved bot applied label for PR's where reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Apr 11, 2019
@benhalpern benhalpern merged commit 5782ffd into forem:master Apr 11, 2019
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: reviewed-approved bot applied label for PR's where reviewer approves changes labels Apr 11, 2019
@rhymes rhymes deleted the rhymes/articles-published-scope branch April 11, 2019 23:31
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.

2 participants