Skip to content

Small tweaks for easier vendoring#3

Merged
hsbt merged 1 commit intoruby:masterfrom
deivid-rodriguez:tweaks-for-easier-vendoring
Apr 18, 2022
Merged

Small tweaks for easier vendoring#3
hsbt merged 1 commit intoruby:masterfrom
deivid-rodriguez:tweaks-for-easier-vendoring

Conversation

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Bundler vendors this file and we have some tools to automatically prepend the Bundler:: namespace so that the vendored version does not collide with the stdlib version.

However, due to how methods are defined, it's hard for our vendoring tool to do the right thing.

I think this makes the code simpler and things easier for us too.

Bundler vendors this file and we have some tools to automatically
prepend the `Bundler::` namespace so that the vendored version does not
collide with the stdlib version.

However, due to how methods are defined, it's hard for our vendoring
tool to do the right thing.

I think this makes the code simpler and things easier for us too.
@hsbt hsbt merged commit 6473bfe into ruby:master Apr 18, 2022
@hsbt
Copy link
Copy Markdown
Member

hsbt commented Apr 18, 2022

@deivid-rodriguez It's reasonable. Thanks.

@nobu
Copy link
Copy Markdown
Member

nobu commented Apr 18, 2022

Why don't replace TSort in instance methods with self.class?

@hsbt
Copy link
Copy Markdown
Member

hsbt commented Apr 18, 2022

@deivid-rodriguez deivid-rodriguez deleted the tweaks-for-easier-vendoring branch April 18, 2022 08:23
@deivid-rodriguez
Copy link
Copy Markdown
Contributor Author

Thank you!

@nobu This patch was enough for our vendoring tool to just work, since previously it would change the code to things like Bundler::TSort.tsort, which... don't really work. So we had to manually nest modules like module Bundler; module TSort; end; end instead of the current style (Module Bundler::TSort; end) so that def TSort.tsort; end would work unchanged.

But... also I think using self.class for instance methods wouldn't work because tsort is mixed into arbitrary classes, yet here we want to delegate to the methods provided by the TSort module, not by the class using tsort.

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.

3 participants