ruby-rdf / rdf-vocab

Common RDF Vocabularies
The Unlicense
50 stars 29 forks source link

Add the frozen_string_literal pragma #35

Closed jcoyne closed 8 years ago

no-reply commented 8 years ago

:+1: from me.

@gkellogg @bendiken we should talk about when and how to do this across the suite. See: https://bugs.ruby-lang.org/issues/11473

jcoyne commented 8 years ago

@no-reply here's how:

find . -name "*.rb" -exec perl -pi -e 'print "# frozen_string_literal: true\n" if $. == 1' {} \;
artob commented 8 years ago

Definitely a :+1: on this count.

no-reply commented 8 years ago

Well, I was all ready to merge, but rbx is failing:

undefined method[]' on nil:NilClass. (NoMethodError)`

no-reply commented 8 years ago

Looks like the rbx problem was just a CI hiccup.

gkellogg commented 8 years ago

Note that the generated vocabulary already freezes strings, so the frozen_string_literal pragma shouldn't cause anything else to freeze. We also can't get rid of the explicit .frozen method on each string because we need to be explicit for Ruby < 2.3.0, but it's certainly a reasonable thing to do, and we should, indeed, look to see where else it may be useful more generically.

jcoyne commented 8 years ago

@gkellogg not all strings were frozen. See the keys: https://github.com/ruby-rdf/rdf-vocab/blob/develop/lib/rdf/vocab/dc.rb#L11-L13

gkellogg commented 8 years ago

True; those should probably be symbols, though. I'll look into that.

no-reply commented 8 years ago

This has me curious whether a symbol like :'dc:creator' instantiates a mutable string literal.

Life's Big Questions :tm:

artob commented 8 years ago

This has me curious whether a symbol like :'dc:creator' instantiates a mutable string literal.

From a theoretical understanding that should just add a new entry to a symbol table. Each time you call #to_s on the Symbol object, it will give you back a newly-allocated String object. Obviously this is implementation-dependent, though.

gkellogg commented 8 years ago

Note that "foo-bar".to_sym #=> :"foo-bar"