Skip to content

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented May 29, 2025

Summary

We should not use puts here, but instead the configured Rails logger. A log level of info is appropriate for all of these instances.

In the Dummy App generator, Rails is not yet loaded, so we use the normal logger instead.

Extracted from #6240

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

@mamhoff mamhoff requested a review from a team as a code owner May 29, 2025 09:41
@github-actions github-actions bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus_promotions Changes to the solidus_promotions gem labels May 29, 2025
@mamhoff mamhoff force-pushed the do-not-use-puts branch from b062230 to 8d4667e Compare May 29, 2025 12:46
@tvdeyen tvdeyen enabled auto-merge May 29, 2025 13:00
@tvdeyen tvdeyen added the backport-v4.5 Backport this pull-request to v4.5 label May 29, 2025
@tvdeyen tvdeyen added backport-v4.1 Backport this pull-request to v4.1 backport-v4.2 Backport this pull-request to v4.2 backport-v4.3 Backport this pull-request to v4.3 backport-v4.4 Backport this pull-request to v4.4 labels May 30, 2025
@tvdeyen tvdeyen force-pushed the do-not-use-puts branch from 8d4667e to 95c2a26 Compare May 30, 2025 06:39
Copy link

codecov bot commented May 30, 2025

Codecov Report

Attention: Patch coverage is 30.76923% with 9 lines in your changes missing coverage. Please review.

Project coverage is 88.82%. Comparing base (6426bb3) to head (0a0313a).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
core/lib/spree/testing_support/common_rake.rb 0.00% 4 Missing ⚠️
core/lib/spree/testing_support/translations.rb 0.00% 2 Missing ⚠️
core/lib/generators/spree/dummy/dummy_generator.rb 0.00% 1 Missing ⚠️
...rs/solidus_promotions/install/install_generator.rb 0.00% 1 Missing ⚠️
...tions/lib/solidus_promotions/promotion_migrator.rb 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6244      +/-   ##
==========================================
- Coverage   88.82%   88.82%   -0.01%     
==========================================
  Files         850      850              
  Lines       18334    18336       +2     
==========================================
+ Hits        16286    16287       +1     
- Misses       2048     2049       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tvdeyen tvdeyen removed backport-v4.1 Backport this pull-request to v4.1 backport-v4.2 Backport this pull-request to v4.2 backport-v4.3 Backport this pull-request to v4.3 backport-v4.4 Backport this pull-request to v4.4 backport-v4.5 Backport this pull-request to v4.5 labels May 30, 2025
@tvdeyen
Copy link
Member

tvdeyen commented May 30, 2025

@tvdeyen tvdeyen added this to the 4.6 milestone May 30, 2025
auto-merge was automatically disabled May 30, 2025 10:23

Head branch was pushed to by a user without write access

@mamhoff mamhoff force-pushed the do-not-use-puts branch from 95c2a26 to 633f6ab Compare May 30, 2025 10:23
We should not use `puts` here, but instead the configured Rails logger.
A log level of `info` is appropriate for all of these instances.

In Generators, we use Thor's `say` method instead.
@mamhoff mamhoff force-pushed the do-not-use-puts branch from 633f6ab to 0a0313a Compare May 30, 2025 12:25
@tvdeyen tvdeyen enabled auto-merge May 30, 2025 12:31
@tvdeyen tvdeyen merged commit 7f2d80a into solidusio:main May 30, 2025
17 of 23 checks passed
@tvdeyen tvdeyen mentioned this pull request May 31, 2025
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_core Changes to the solidus_core gem changelog:solidus_promotions Changes to the solidus_promotions gem
Projects
Development

Successfully merging this pull request may close these issues.

3 participants