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

[Source] Autoload fileutils #1906

Merged
merged 1 commit into from May 11, 2017
Merged

[Source] Autoload fileutils #1906

merged 1 commit into from May 11, 2017

Conversation

segiddins
Copy link
Member

Description:

Since fileutils is now a gem

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends

I will abide by the code of conduct.

Since fileutils is now a gem
@segiddins
Copy link
Member Author

Any objection to this?

@hsbt
Copy link
Member

hsbt commented Apr 28, 2017

I wonder why we use autoload for default gem.

PS. Matz is against to use autoload. https://bugs.ruby-lang.org/issues/5653. We have the plan to deprecate or remove on long term (at Ruby 3.0?)

@segiddins
Copy link
Member Author

The removal of autoload would make bundler and rubygems setup much much slower, and I think autoload is far preferable to manually adding require "foo" to every place foo is used to lazy-load the code (in addition to being faster, because of the way RubyGem's require is implemented)

@djberg96
Copy link
Contributor

@hsbt Generally true that it should be avoid because of threading issues from what I've read. But, rubygems isn't typically used as a library, so I'm not sure how much real risk there is.

@matthewd
Copy link
Contributor

@djberg96 FYI autoload has officially been threadsafe since 2.0 (ruby/ruby@1e7f99d)... though I've seen hints of bugs, so whether that's informative may depend on how recent your existing information is. 😅

(Since that change, I'm not sure why it would still be removed, tbh.. but that's a discussion for elsewhere.)

@djberg96
Copy link
Contributor

@matthewd Ok, thanks. Alright, then I say merge.

@segiddins
Copy link
Member Author

@homu r+

@homu
Copy link
Contributor

homu commented May 11, 2017

📌 Commit 535d405 has been approved by segiddins

@homu
Copy link
Contributor

homu commented May 11, 2017

⌛ Testing commit 535d405 with merge 6d86cd4...

homu added a commit that referenced this pull request May 11, 2017
…dins

[Source] Autoload fileutils

# Description:

Since fileutils is now a gem

# Tasks:

- [ ] Describe the problem / feature
- [ ] Write tests
- [ ] Write code to solve the problem
- [ ] Get code review from coworkers / friends

I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
@homu
Copy link
Contributor

homu commented May 11, 2017

☀️ Test successful - status

@homu homu merged commit 535d405 into master May 11, 2017
@segiddins segiddins deleted the seg-source-autoload-fileutils branch September 5, 2017 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants