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

Remove unnecessary encoding comment #6513

Merged
merged 2 commits into from Nov 3, 2017

Conversation

Projects
None yet
4 participants
@ashmaroli
Member

ashmaroli commented Nov 3, 2017

Rubocop did not catch this superfluous magic comment because it was on the second line which was not preceded by a line with a comment starting with a shebang

@DirtyF

DirtyF approved these changes Nov 3, 2017

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Nov 3, 2017

Member

@ashmaroli There might be a last encoding comment to delete:

ag "Encoding: utf-8"
lib/jekyll/utils/ansi.rb
3:# Encoding: utf-8
Member

DirtyF commented Nov 3, 2017

@ashmaroli There might be a last encoding comment to delete:

ag "Encoding: utf-8"
lib/jekyll/utils/ansi.rb
3:# Encoding: utf-8

@DirtyF DirtyF added the internal label Nov 3, 2017

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Nov 3, 2017

Member

There might be a last encoding comment to delete:

Right. That was not detected by Rubocop either.. 😏
Thanks for catching that.. will push a commit shortly..

Member

ashmaroli commented Nov 3, 2017

There might be a last encoding comment to delete:

Right. That was not detected by Rubocop either.. 😏
Thanks for catching that.. will push a commit shortly..

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Nov 3, 2017

Member

@jekyllbot: merge +dev

Member

DirtyF commented Nov 3, 2017

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 3838564 into jekyll:master Nov 3, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Nov 3, 2017

Member

What’s with the red X?

Member

pathawks commented Nov 3, 2017

What’s with the red X?

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF
Member

DirtyF commented Nov 3, 2017

@pathawks fixed in 94dc926

@ashmaroli ashmaroli deleted the ashmaroli:encoding-comment branch Nov 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment