Tech decision: frozen_string_literal

Tech
1

we recently added “# frozen_string_literal: true” as a rule to rubocop.
There are benefits to this but I think the process is too time consuming.
Step 1 - make a PR and forget about the magic comment
Step 2 - look at code climate and see the comment is missing
Step 3 - add comment to file/files, commit and push
Step 4 - verify code climate is clear

This will need to be repeated for hundreds of files… I think this is too much time.
What do you guys think?
I see 4 options:
A - disable the rule for now
B - keep the rule but allow ourselves to ignore this rule if we want
C - keep the rule but make an issue to add the comment to most files in one go
D - where we are now, keep the rule and add comment to individual files as we change them

Thoughts? Votes?

I dont think this is important enough for the time/effort it takes. So I vote for option A, or if the group decides to keep the rule, I’d go for option B.

2

I have been struggling with this decision as well. It’s a lot of work and clutter to add this to every file. But I believe that the result would be good. I would like to keep the rule. But this isn’t a very strong desire.

While we configured Rubocop to ignore existing files, Code Climate complains for every changed file. Configuring Code Climate for this would probably mean an additional config file again.

I would propose the following:

  • We keep the rule.
  • We allow ourselves to ignore Code Climate for this rule.
  • We try to update some files in batches, e.g. all specs. But we have to be careful as this can break code. That’s why I think we shouldn’t try to do all files at once.
3

If we decided to add the rule I’d try C and see how much can get done without breaking things and do the rest in batches.

I think ignoring the rule i’d be also annoying as someone will have to go approve the PR in CodeClimate.

If all this sounds too much. I’d go with A. We have much bigger problems in terms of performance IMO.

4

I found the first case where we need to use

frozen_string_literal: false

https://semaphoreci.com/openfoodfoundation/openfoodnetwork-2/branches/pull-request-4621/builds/35

This topic is taking way too much of my time (it comes up in basically all PRs).
I would like to disable this rule completely for now if you guys dont disagree. It’s a nice to have.

5

fine for me but I haven’t found any problem with it. It takes me less than 1min to address it when I see CodeClimate complaining.