You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Tao Li <to...@gmail.com> on 2022/05/12 05:04:51 UTC
[DISCUSS] Disable Style/FrozenStringLiteralComment for ruby
Hi team,
By default Style/FrozenStringLiteralComment is enabled in rubocop. If we
update a ruby file, rubocop prompts `Missing frozen string literal comment`
(see
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt)
<https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt>
.
To address this warn, we need to add `# frozen_string_literal: true` to the
top of the ruby file(see
https://github.com/rubocop/rubocop/blob/master/config/default.yml#L3631),
which will be added to the top of the `Apache License` and will look
strange.
I don't think this `FrozenStringLiteralComment` check is very necessary.
Can we disable it?
I filed a JIRA: https://issues.apache.org/jira/browse/HBASE-27026 to track
the issue and put a PR https://github.com/apache/hbase/pull/4423.
Welcome to discuss whether this approach is reasonable or not.
Thanks,
Tao Li
Re: [DISCUSS] Disable Style/FrozenStringLiteralComment for ruby
Posted by Tao Li <to...@gmail.com>.
Hi Mike,
Thank you for your reply.
I add `# frozen_string_literal: true` after the license header and RuboCop
is not happy. I did the verification in this PR
https://github.com/apache/hbase/pull/4416, you can refer to it.
This comment (while no longer the default in Ruby 3) may indeed provide
some minor performance improvements. Ruby is used in HBase in only a small
number of scenarios. So I think the potential performance gains are limited.
But if we can only add it before the license header, it looks a little
weird. Maybe it depends on how we weigh it. Thank you again.
Mike Drob <md...@apache.org> 于2022年5月13日周五 20:38写道：
>
>
> On 2022/05/12 05:04:51 Tao Li wrote:
> > Hi team,
> >
> > By default Style/FrozenStringLiteralComment is enabled in rubocop. If we
> > update a ruby file, rubocop prompts `Missing frozen string literal
> comment`
> > (see
> >
> https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt
> )
> > <
> https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt
> >
> > .
> >
> >
> > To address this warn, we need to add `# frozen_string_literal: true` to
> the
> > top of the ruby file(see
> > https://github.com/rubocop/rubocop/blob/master/config/default.yml#L3631
> ),
> > which will be added to the top of the `Apache License` and will look
> > strange.
> >
>
> I think this line can be applied after the license header and still be
> compliant with RuboCop.
>
> >
> > I don't think this `FrozenStringLiteralComment` check is very necessary.
> > Can we disable it?
> >
>
> Using frozen strings (while no longer the default in Ruby 3) has some
> minor performance improvements where it is applied.
>
> >
> > I filed a JIRA: https://issues.apache.org/jira/browse/HBASE-27026 to
> track
> > the issue and put a PR https://github.com/apache/hbase/pull/4423.
> >
> > Welcome to discuss whether this approach is reasonable or not.
> >
> >
> >
> > Thanks,
> >
> > Tao Li
> >
>
Re: [DISCUSS] Disable Style/FrozenStringLiteralComment for ruby
Posted by Mike Drob <md...@apache.org>.
On 2022/05/12 05:04:51 Tao Li wrote:
> Hi team,
>
> By default Style/FrozenStringLiteralComment is enabled in rubocop. If we
> update a ruby file, rubocop prompts `Missing frozen string literal comment`
> (see
> https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt)
> <https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4416/2/artifact/yetus-general-check/output/diff-patch-rubocop.txt>
> .
>
>
> To address this warn, we need to add `# frozen_string_literal: true` to the
> top of the ruby file(see
> https://github.com/rubocop/rubocop/blob/master/config/default.yml#L3631),
> which will be added to the top of the `Apache License` and will look
> strange.
>
I think this line can be applied after the license header and still be compliant with RuboCop.
>
> I don't think this `FrozenStringLiteralComment` check is very necessary.
> Can we disable it?
>
Using frozen strings (while no longer the default in Ruby 3) has some minor performance improvements where it is applied.
>
> I filed a JIRA: https://issues.apache.org/jira/browse/HBASE-27026 to track
> the issue and put a PR https://github.com/apache/hbase/pull/4423.
>
> Welcome to discuss whether this approach is reasonable or not.
>
>
>
> Thanks,
>
> Tao Li
>