Skip to content
Permalink
Browse files
Convert template to US-ASCII to fix error
I introduced some tests in a feature branch to match the contents of
`/etc/nginx/router_routes.conf`. They worked fine when run with `bundle exec
rake spec` or `bundle exec rspec modules/router/spec`. But when run as
`bundle exec rake` each should block failed with:

    ArgumentError:
      invalid byte sequence in US-ASCII

I eventually found that removing the `.with_content(//)` matchers made the
errors go away. That there weren't any weird characters in the spec file. And
that it could be reproduced by requiring Puppet in the same interpreter with:

    rake -E 'require "puppet"' spec

That particular template appears to be the only file in our codebase with an
identified encoding of `utf-8`. All others are `us-ascii`:

    dcarley-MBA:puppet dcarley$ find modules -type f -exec file --mime {} \+ | grep utf
    modules/router/templates/routes.conf.erb:                                         text/plain; charset=utf-8

Attempting to convert that file back to US-ASCII identified the offending
character as something that looked like a whitespace:

    dcarley-MBA:puppet dcarley$ iconv -f UTF8 -t US-ASCII modules/router/templates/routes.conf.erb 2>&1 | tail -n5
      proxy_intercept_errors off;

      # Set proxy timeout to 50 seconds as a quick fix for problems
      #
    iconv: modules/router/templates/routes.conf.erb:458:3: cannot convert

After replacing it (by hand) the file identifies as `us-ascii` again:

    dcarley-MBA:puppet dcarley$ file --mime modules/router/templates/routes.conf.erb
    modules/router/templates/routes.conf.erb: text/plain; charset=us-ascii

Now the tests work! One hour of my life I won't get back..
  • Loading branch information
dcarley committed May 1, 2013
1 parent bfe3f64 commit 63b36f93bf75a848e2125008aa1e880c5861cf46
Showing with 1 addition and 1 deletion.
  1. +1 −1 modules/router/templates/routes.conf.erb
@@ -460,7 +460,7 @@ location ~ ^/apply-for-a-licence/? {
proxy_intercept_errors off;

# Set proxy timeout to 50 seconds as a quick fix for problems
# where civica QueryPayments calls are taking too long.
# where civica QueryPayments calls are taking too long.
proxy_read_timeout 50;
proxy_pass http://varnish;
}

15 comments on commit 63b36f9

@nikialeksey

This comment has been minimized.

Copy link

@nikialeksey nikialeksey replied Oct 19, 2019

Did you think it is searchable message? Why didnt you write a doc? Or asked and answered in SO (it could be more usefull)? But you wrote the story with explanation in the commit message, come on, and after 6 years one man told it was great commit message... Are you crazy guys? @dcarley @fatbusinessman

Commit message with the long explanation helps nobody, because it does not searchable by google, even by github. Event project people dont know about it, because how many programmers use search by commit message when they got an error? I will answer for you - nobody. Even if we assume that project people can see the message by scrolling git history there is 27k commit, guys. You really help nobody, and @fatbusinessman wrote an article only for make this commit searchable. If the commit message require a blog post for being good commit message - it is poor commit message.

@celtric

This comment has been minimized.

Copy link

@celtric celtric replied Oct 19, 2019

What you're describing is a deficiency of the search engines, not the commit itself right @nikialeksey?

@nikialeksey

This comment has been minimized.

Copy link

@nikialeksey nikialeksey replied Oct 19, 2019

@celtric I think that large commit message helps nobody, never. Doc helps, answer on SO helps, readme helps, but long commit message in the commit with only one character changed... No one needs to it, moreover no one will ever see it.

Ok, what do you know about maintainability? Imagine if there is another person in a project (event in this project) who face the challenge US-ASCII error, how he can find this commit message which obviously can help? Usually, a programmer googles error and find the answer, but in our case he will not find this commit message. So, this commit message does not improve maintainability.

Commit message is not intended to documentation, it will not be formatted, it will not be indexed by search engine, there is no helpful system around it for reading such documentation in commit messages...

Even writing the test (with verifying encoding) would be better, because that guy (who will face the challenge US-ASCII error) will se the broken test and understand what happened.

@celtric

This comment has been minimized.

Copy link

@celtric celtric replied Oct 19, 2019

@celtric I think that large commit message helps nobody, never. Doc helps, answer on SO helps, readme helps, but long commit message in the commit with only one character changed... No one needs to it, moreover no one will ever see it.

I disagree depending on the context. I've found descriptive commit message that were very useful when trying to understand why a change was made, as they provided the "why". These messages where right in the place where I would have expected them to be (close to the change).

You suggest:

  • Documentation (in an external wiki?): I would've had to change context (my IDE) to get the information. Also, how would I get to that documentation? A reference in the commit? Why not inline the content?
  • Documentation (in the form of a comment next to the change): for a fix like this there's no point in leaving a comment in the code as it's relevance is temporal (no one will care next week, issue is now fixed).
  • Answer on SO: yes, that would be helpful. At the same time, if every time you commit a fix and want to explain the why you have to go on SO, we would not get anything done.
  • README: again, the scope of the comment is very specific. Why should we add noise in the README?

Although you generalise (or at least I understand that) that "all large commits" are unhelpful, I suppose you're actually talking about commit messages that provide context that you would never care about unless you're suffering the same issue again. That is, a person working in this codebase wouldn't care why a space was added, but someone with the US-ASCII error would.

Ok, what do you know about maintainability? Imagine if there is another person in a project (event in this project) who face the challenge US-ASCII error, how he can find this commit message which obviously can help? Usually, a programmer googles error and find the answer, but in our case he will not find this commit message. So, this commit message does not improve maintainability.

Commit message is not intended to documentation, it will not be formatted, it will not be indexed by search engine, there is no helpful system around it for reading such documentation in commit messages...

We agree then that we're not talking about all large commits, just these ones, right? Again, IMHO the problem are the tools (search engines, GitHub, IDEs, etc.) not the commits message itself. Isn't it ridiculous that we cannot find text inside commit messages? In a few years we'll look back and consider this situation ridiculous, and the commit message would not have changed (the tools would).

Even writing the test (with verifying encoding) would be better, because that guy (who will face the challenge US-ASCII error) will se the broken test and understand what happened.

I think you're right, only partially. If this project's focus was more on the templating itself I would agree, but the core of the project is not about this error, it just happened to suffer it.

@SeinopSys

This comment has been minimized.

Copy link

@SeinopSys SeinopSys replied Oct 19, 2019

Likely reason for the sudden activity if anyone's wondering: https://reddit.com/r/programming/comments/djnp8k/my_favourite_git_commit/

@krmbzds

This comment has been minimized.

Copy link

@krmbzds krmbzds replied Oct 19, 2019

I use a Karabiner rule on macOS to solve this. Hope this helps a passerby.

@QazerLab

This comment has been minimized.

Copy link

@QazerLab QazerLab replied Oct 25, 2019

I think that large commit message helps nobody, never.

Large commit messages help at least me (so your theorem is disproved).

Every time I use things like git log --grep, git log -S, git log -G, git bisect I see commit messages as the first meaningful output of git, and as long as these messages explain the motivation of the commit, they are highly useful.

By the way, git bisect is an easy way to

see the message by scrolling git history there is 27k commit, guys.

when you do troubleshooting.

@vchslv13

This comment has been minimized.

Copy link

@vchslv13 vchslv13 replied Oct 25, 2019

Thanks for git bisect, @QazerLab. Was doing it manually once to find a bug, didn't know that git helps to automate this :)

@ganqqwerty

This comment has been minimized.

Copy link

@ganqqwerty ganqqwerty replied Oct 25, 2019

This commit is so awesome that there is a blogpost about it was translated and ended ip in Russian Medium! @dcarley wait for more noobs from Ural who will teach you how to live!

@nikialeksey

This comment has been minimized.

Copy link

@nikialeksey nikialeksey replied Oct 26, 2019

@dcarley @fatbusinessman I'm sorry if I offended you. My english is far from being good. I made a mistake I wrote my comments here, I should have to write my commens somewhere on reddit, hackernews or "Russian Medium". Sorry for that.

I wrote my own blog post about long commit messages (in Russain). It contains all my thoughts about this case and long commit messages.

@yatsenko-ihor

This comment has been minimized.

Copy link

@yatsenko-ihor yatsenko-ihor replied Oct 27, 2019

Did you think it is searchable message? Why didnt you write a doc? Or asked and answered in SO (it could be more usefull)? But you wrote the story with explanation in the commit message, come on, and after 6 years one man told it was great commit message... Are you crazy guys? @dcarley @fatbusinessman

Commit message with the long explanation helps nobody, because it does not searchable by google, even by github. Event project people dont know about it, because how many programmers use search by commit message when they got an error? I will answer for you - nobody. Even if we assume that project people can see the message by scrolling git history there is 27k commit, guys. You really help nobody, and @fatbusinessman wrote an article only for make this commit searchable. If the commit message require a blog post for being good commit message - it is poor commit message.

Your message looking strange, if you criticize - offer. Why you didn't provide a link to SO, but criticizing this guy. He did really useful work but didn't tell anyone. You are just left an unuseful comment and that's it.

@yatsenko-ihor

This comment has been minimized.

Copy link

@yatsenko-ihor yatsenko-ihor replied Oct 27, 2019

Did you think it is searchable message? Why didnt you write a doc? Or asked and answered in SO (it could be more usefull)? But you wrote the story with explanation in the commit message, come on, and after 6 years one man told it was great commit message... Are you crazy guys? @dcarley @fatbusinessman
Commit message with the long explanation helps nobody, because it does not searchable by google, even by github. Event project people dont know about it, because how many programmers use search by commit message when they got an error? I will answer for you - nobody. Even if we assume that project people can see the message by scrolling git history there is 27k commit, guys. You really help nobody, and @fatbusinessman wrote an article only for make this commit searchable. If the commit message require a blog post for being good commit message - it is poor commit message.

Your message looking strange, if you criticize - offer. Why you didn't provide a link to SO, but criticizing this guy. He did really useful work but didn't tell anyone. You are just left an unuseful comment and that's it.

Post on SO below not so good, but I'm lazy today to make it better. The main idea, that's all main info(error message output) will be indexable.
https://stackoverflow.com/questions/58583380/argumenterror-invalid-byte-sequence-in-us-ascii-argument-error-when-i-run-bun/58583429#58583429

@zetroot

This comment has been minimized.

Copy link

@zetroot zetroot replied Oct 29, 2019

From now on this commit definitely has the biggest recongnition/size ratio!

@evandrix

This comment has been minimized.

Copy link

@evandrix evandrix replied Mar 8, 2020

bump

@wozniak

This comment has been minimized.

Copy link

@wozniak wozniak replied Jul 15, 2020

The only soul who can write commit messages like that.

Please sign in to comment.