Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the test suite compatible with --enable-frozen-string-literal #120

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

casperisfine
Copy link
Contributor

Ref: https://bugs.ruby-lang.org/issues/20205

Since rexml is tested as part of ruby-core CI, it needs to be compatible with the --enable-frozen-string-literal option.

@kou
Copy link
Member

kou commented Mar 15, 2024

Could you also add a CI job with --enable-frozen-string-literal?

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 94a116a..af042af 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -33,6 +33,18 @@ jobs:
       - name: Test
         run: bundle exec rake test
 
+  frozen-string-literal:
+    name: frozen-string-literal
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: ruby/setup-ruby@v1
+        with:
+          ruby-version: ruby
+          bundler-cache: true
+      - name: Test
+        run: bundle exec ruby --frozen-string-literal -S rake test
+
   gem:
     name: "Gem: ${{ matrix.ruby-version }} on ${{ matrix.runs-on }}"
     runs-on: ${{ matrix.runs-on }}

Ref: https://bugs.ruby-lang.org/issues/20205

Since `rexml` is tested as part of ruby-core CI, it needs to be compatible
with the `--enable-frozen-string-literal` option.
@casperisfine casperisfine force-pushed the enable-frozen-string-literal branch from 4d1e219 to 240166b Compare March 15, 2024 13:04
@casperisfine
Copy link
Contributor Author

Done.

@kou kou merged commit d4e79f2 into ruby:master Mar 15, 2024
40 checks passed
@kou
Copy link
Member

kou commented Mar 15, 2024

Thanks.

@casperisfine
Copy link
Contributor Author

Thanks for the merge ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants