premailer / css_parser

Ruby CSS Parser
Other
279 stars 110 forks source link

Add rubocop #114

Closed ojab closed 3 years ago

ojab commented 3 years ago

Branch on top of the https://github.com/premailer/css_parser/pull/113, will squash all rubocop commits & replace Travis badge with GH Actions one before merging.

I didn't squashed it yet since I'm not sure that all changes are acceptable and I'm ready to remove/change some stuff if needed and it's easier to do with separate commits.

Squashed, badge is here, so probably ready.

ojab commented 3 years ago

Oh you already merged last one,let me find where is the GH Actions badge and fix rubocop CI.

grosser commented 3 years ago

prefer to leave string quotes alone to reduce change

grosser commented 3 years ago

yeah

On Fri, Jan 8, 2021 at 9:30 AM Slava Kardakov notifications@github.com wrote:

@ojab commented on this pull request.

In .github/workflows/ci.yml https://github.com/premailer/css_parser/pull/114#discussion_r554088197:

@@ -25,3 +25,19 @@ jobs: bundler-cache: true

   - run: bundle exec rake

+

  • rubocop:
  • name: Run rubocop
  • runs-on: ubuntu-latest
  • steps:
    • uses: actions/checkout@v2
    • run: rm Gemfile.lock

Yep, not sure what's desired policy. Just to have locked version and update it manually time to time?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/premailer/css_parser/pull/114#discussion_r554088197, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACYZ6TFONNYZJ5E42WW2LSY46J3ANCNFSM4V2XB7YA .

grosser commented 3 years ago

meh, don't care, just looked odd

On Fri, Jan 8, 2021 at 9:44 AM Slava Kardakov notifications@github.com wrote:

@ojab commented on this pull request.

In test/test_rule_set_expanding_shorthand.rb https://github.com/premailer/css_parser/pull/114#discussion_r554095468:

   shorthand = "list-style: #{type} inside url('chess.png');"

declarations = expand_declarations(shorthand) assert_equal(type, declarations['list-style-type']) end end

  • protected +protected

Yep, private/protected is the same for rubocop and it' not indented everywhere else. Looks funky here since it's top-level class and there is no additional indent. I could either change it to two spaces everywhere or just exclude ./test/*/, whatever fits you.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/premailer/css_parser/pull/114#discussion_r554095468, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACYZ6R6OLZEJVPLD533N3SY476VANCNFSM4V2XB7YA .

ojab commented 3 years ago

prefer to leave string quotes alone to reduce change

I suppose it's about interpolation, since there are only 3 " -> ' changes if we exclude gemspec.