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

Fix NameError by adding ::operator #3118

Merged
merged 1 commit into from Apr 1, 2023

Conversation

ninoseki
Copy link
Contributor

@ninoseki ninoseki commented Apr 1, 2023

Description

Closes #3117 by adding :: operator.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@MSP-Greg MSP-Greg added the bug label Apr 1, 2023
@MSP-Greg
Copy link
Member

MSP-Greg commented Apr 1, 2023

Verified bug as shown in #3117 with Ubuntu 22.04, Ruby master/head, Puma master.

EDIT:

@ninoseki - Thank you for the PR and the example in #3117. I knew there have been changes re top-level constants, but not the kind of change shown here...

@MSP-Greg MSP-Greg merged commit 6b7b9f8 into puma:master Apr 1, 2023
64 of 65 checks passed
@dentarg
Copy link
Member

dentarg commented Apr 1, 2023

We must be missing a test case?

MSP-Greg added a commit to MSP-Greg/puma that referenced this pull request Apr 1, 2023
MSP-Greg added a commit to MSP-Greg/puma that referenced this pull request Apr 1, 2023
@MSP-Greg MSP-Greg mentioned this pull request Apr 1, 2023
7 tasks
@MSP-Greg
Copy link
Member

MSP-Greg commented Apr 1, 2023

@dentarg

Good idea. See #3119. Fails if this is reverted, as below:

  1) Error:
TestRackUp::TestOnBootedHandler#test_on_booted:
NameError: uninitialized constant Puma::Rack::CommonLogger
    /mnt/c/Greg/GitHub/puma/lib/rack/handler/puma.rb:35:in `block in config'
    /mnt/c/Greg/GitHub/puma/lib/puma/configuration.rb:195:in `configure'
    /mnt/c/Greg/GitHub/puma/lib/puma/configuration.rb:188:in `initialize'

@dentarg
Copy link
Member

dentarg commented Apr 1, 2023

Excellent, thanks for that @MSP-Greg ❤️

MSP-Greg added a commit that referenced this pull request Apr 1, 2023
@ninoseki ninoseki deleted the fix-common-logger-issue branch April 2, 2023 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to initialize Rack::CommonLogger
3 participants