sinatra / mustermann

your personal string matching expert
http://sinatrarb.com/mustermann/
MIT License
664 stars 63 forks source link

Use String.new instead of quotes #110 #122

Closed michal-granec closed 2 years ago

michal-granec commented 2 years ago

Motivation

Mustermann is not compatible with --enable=frozen-string-literal. Running with such option raises Runtime error:

FrozenError:
       can't modify frozen String: ""

Issue

https://github.com/sinatra/mustermann/issues/110

Changes

Addition notes

Command I have used to run specs (in project root directory):

RUBYOPT=--enable=frozen-string-literal bundle exec rspec

There are unfortunatelly 2 additional things I had to locally fix in the dependencies to make all the test passing:

  1. Hansi::StringRendered#L38 is also working with frozen string literal. This is not an issue since this is dependencie of mustermann-contrib :) If it makes sense I can go there and make another PR.
  2. Rack::MockRequest#L125 is also working with string that is frozen. This has been changed in versions >= 2.0.0 to use String.new. So as soon as sinatra dependency is updated this wont be any problem. This is also not an issue since this is used only in tests.
michal-granec commented 2 years ago

@olleolleolle should I do something after your approval :) or just wait for you to merge it?

olleolleolle commented 2 years ago

@namusyaka 👋 This looks good - right?!

Edit: I don't have merge privileges, but I like this change.

jkowens commented 2 years ago

@michal-granec can you open a PR to fix Hansi? Thanks!

michal-granec commented 2 years ago

@jkowens will do :+1: