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

Puma loading, History.md questions #2315

Closed
MSP-Greg opened this issue Jul 21, 2020 · 3 comments
Closed

Puma loading, History.md questions #2315

MSP-Greg opened this issue Jul 21, 2020 · 3 comments
Labels

Comments

@MSP-Greg
Copy link
Member

  1. Looking at the issues re loading in Puma. For instance, building Puma without SSL (see Allow Puma to compile when built without SSL, load SSL files on demand #2305), and also JSON/OpenSSL/YAML loading (see Load JSON at runtime #2269)

Not sure how far to try and take minimal loading, but several files may not need to be loaded unless binding to SSL sockets, JSON/Psych/YAML may not need to be loaded, and some engines/platforms cannot use workers/cluster.

Also, given that Puma is relatively 'flat' re namespaces, loading could be centralized, maybe with a specific file like puma/load.rb. It could load all files needed by all configurations, and with comments make clear any files that are loaded outside of it. Some may work best with autoload, and maybe all other Puma files can be loaded with require_relative. By using one file, it may help remind everyone that any require statements added in PR's/commits should be reviewed.

Having also been in CI a bit lately, I always dislike having requires in test files, as it may hide issues with proper loading. Given that CI uses a fair bit of spawned processes to launch Puma, that isn't as much of an issue as with other repos...

Anyway, looking for feedback, thoughts, etc.

  1. Small point, but the History.md file. Should newest go on top or bottom? I recently added an item in the wrong place, decided to move it, and also wanted to update PR Move initialization of KeyManagerFactory, TrustManagerFactory to server #2302, and itvolved History.md...
@MSP-Greg
Copy link
Member Author

Re History.md, any objections to the following? Added linking to PR's:

https://github.com/MSP-Greg/puma/blob/history/History.md

If the most recent release is on the top, seems like the most recent commits/PR's should also go on top?

@dentarg
Copy link
Member

dentarg commented Oct 1, 2020

Solved by #2305 and #2383 @MSP-Greg?

@MSP-Greg
Copy link
Member Author

MSP-Greg commented Oct 1, 2020

LGTM

@MSP-Greg MSP-Greg closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants