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

http11: Remove qsort code paths [changelog skip] #2073

Merged
merged 2 commits into from Nov 13, 2019

Conversation

AnthonyClark
Copy link
Contributor

@AnthonyClark AnthonyClark commented Nov 12, 2019

Description

I was just trying to familiarize myself a bit with puma and was trying to track down the usage of the code behind the if defined(HAVE_QSORT_BSEARCH). Looks like it was added back to mongrel in engineyard/mongrel@cd44d2e and then added to puma in 7c9d988 but I don't see it being used.

Looks like it was removed form the Unicorn web server in https://bogomips.org/unicorn.git/commit/?id=9625ad39b73d3d1443ff097e9113d1ec9c9d5f00

Of course this isn't a super helpful change if there's plans to move from the c extension entirely (#1889)..

It doesn't look like there was any test coverage so I didn't add/remove there, but not skipping CI since it is a code change.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] to all commit messages.
  • I have added 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.

@dentarg
Copy link
Member

dentarg commented Nov 12, 2019

I have added [changelog skip] to all commit messages

The check only looks in the pull request title

@AnthonyClark AnthonyClark changed the title http11: Remove qsort code paths http11: Remove qsort code paths [changelog skip] Nov 12, 2019
[changelog skip]

Had the following warning during compilation without the include:
warning: implicitly declaring library function 'isspace' with type 'int (int)'
@nateberkopec
Copy link
Member

#1889 is very long term, more of an expression of my wish to narrow the scope of the native extensions. This is a great PR.

@nateberkopec nateberkopec merged commit befe00a into puma:master Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants