search

bridgetownrb / bridgetown

A next-generation progressive site generator & fullstack framework, powered by Ruby
https://www.bridgetownrb.com
MIT License
1.14k stars 114 forks source link

Spike: find a suitable Inflector #876

Closed jaredcwhite closed 5 months ago

jaredcwhite commented 5 months ago

Details forthcoming…this is a placeholder item to determine which inflector to use as we're removing Active Support dependencies.

Some possible options:

jclusso commented 5 months ago

How is removing ActiveSupport moving this project forward? It's the 5th most popular gem and is extremely stable. I could understand the idea if it was to eliminate a dependency, or bring better functionality. This is just replacing one thing with another for no reason. This is quite a disappointment.

jaredcwhite commented 5 months ago

@jclusso Dang, I was hoping this conversation had been resolved some time ago? I'm not sure why we're covering the same ground again.

ICYMI and need more context: https://www.bridgetownrb.com/future/road-to-bridgetown-2.0-escaping-burnout/

jclusso commented 5 months ago

Resolved implies I think this is a good decision...

If someone is looking for this functionality in the future, it should be known that it did exist, but was abandoned on principle, not due to the functionality.

Also, I think it would be interesting to poll the Bridgetown community and see if they support removing Active Support. Your post you linked about 2.0 speaks directly to the lack of that being done when removing Typescript.

brandondrew commented 5 months ago

Jarrett makes a very good point. If you (@jaredcwhite) think DHH's removal of Typescript was a violation of the Code of Conduct, why are you doing the same thing?

ayushn21 commented 5 months ago

If removing ActiveSupport proves tricky or time consuming, I'd be in favour of vendoring it rather than dropping it completely. That said, @jclusso @brandondrew do you have any technical concerns regarding this change?

From my POV, ActiveSupport is quite a large, monolithic gem with a large surface area. That's fantastic for Rails. For Bridgetown I wouldn't mind slimming down the dependencies a bit and pull in just the functionality we need.

jclusso commented 5 months ago

@ayushn21 as previously stated I'm all for removing an unecessary dependency or one that provides little value. This however is for the agenda to "de-37signals-ify" Bridgetown. It's based on opinions of DHH's actions on Turbo, a completely unrelated project.

The reason to remove Active Support is based on not wanting to tangentially include code from a project that some believe violates the CoC. I feel like vendoring it is the equivelent of saying you don't support theives, but buying their discount stolen products. I don't see how we can vendor it in good faith.

Also, this feels like a slippery slope. Have we fully vetted the entire Gemfile.lock that pulls in like 100 other dependencies to make sure they're up to par with the CoC? How far does it go?

ayushn21 commented 5 months ago

@jclusso It's true it's pretty fiddly philosophical territory here. It's tenuous ground and we're unlikely to agree with each other completely.

However I feel we as project maintainers still have the right to set the project's direction and philosophy. I don't want to speak for Jared here, but I think he'd agree with me when I say we remain completely open to discussing technical grounds on which we shouldn't make a change.

If there's little to no effect on Bridgetown users and the effort to make the change is put in by the maintainers, I'm struggling to see the issue here.

I'll say again that we're completely open to feedback regarding any technical problems this may cause. Please share any concrete difficulties you would encounter should we drop ActiveSupport for whatever else. We'll address them before we choose what path to go down.

jclusso commented 5 months ago

@ayushn21 one potential concern I have is we rely on https://github.com/dvmonroe/sanity-ruby for integrating our CMS. This uses Active Support. I'm sure many other projects include AS due to how common it is.

Should we start vendoring or reinventing bits and pieces of AS, we run the risk of unexpected issues. Bugs like this could be tricky to identify, especially to someone that isn't completely aware that AS is being vendored or bits of it were recreated.

I don't have any concrete examples besides that. There are numerous bits and pieces of AS that are useful though. It's not always apparent something relies on AS and removing it might comprimise the stability of the project, especially if some bits are here, but others aren't.

I agree that project maintainers hold the sole right to make any decision they like. If I come up with any other items, I'll share those here.

Lastly, I know I've made a few contributions to Bridgetown already that used AS. Being able to reach for a poweful tool to add new functionality, is super convenient.

ayushn21 commented 5 months ago

Thanks for sharing @jclusso :)

I'm not that familiar with Bridgetown's usage of AS under the hood so I haven't got a response off the top of my head. I'll let @jaredcwhite address that when he has some time and will do my own research as and when I get time.

jaredcwhite commented 5 months ago

@jclusso:

Also, this feels like a slippery slope. Have we fully vetted the entire Gemfile.lock that pulls in like 100 other dependencies to make sure they're up to par with the CoC? How far does it go?

Please let me know which gems are owned by people violating their own CoCs and I will gladly take a close look. There's no slippery slope here!

The reason to remove Active Support is based on not wanting to tangentially include code from a project that some believe violates the CoC. I feel like vendoring it is the equivelent of saying you don't support theives, but buying their discount stolen products. I don't see how we can vendor it in good faith.

Vendoring is merely a near-term solution, I'll grant you that. The long-term solution is to migrate off completely. This is certainly no easy feat and will take some real time and effort. For example, it only just dawned on me the other day that the CLI framework we use, Thor, is completely located within the Rails repo! It was actually migrated from I believe wycats (aka Yehuda Katz, the original author) some while back. I honestly have no idea at the present moment how to deal with that.

Regarding Active Support specifically, it's obviously not going to blink out of existence right away. We'll whittle away at it a bit for v2, some more for v2.1, etc. And long-term, here's what I can promise you. If you want to require "active_support" in your own codebase and use any or all AR features, that's totally cool! Nobody here will bat an eye. And if doing so causes some weird compatibility glitch at some point, I'm absolutely in favor of filing issues and dealing with it.

But expecting us to use AS in perpetuity merely because it's convenient or popular is a huge leap. Other Ruby web frameworks don't use it. Hanami doesn't use it. Roda doesn't use it. Sinatra doesn't use it. Sidekiq, a very popular background jobs processor, doesn't use it.

The reason Bridgetown uses it is because when the project started in 2000, the events in question re: 37signals & CoCs hadn't transpired yet, I was still a happy Rails user, and Active Support was indeed very useful and convenient. But I was already aware at that time that many fine Rubyists out there have real issues with how much AS affects the Ruby runtime and the amount of monkey-patching involved. That didn't bother me enough to avoid using it. But here we are.

As @ayushn21 said, while we may not reach agreement on the philosophy of this, we very much want to avoid suddenly breaking any user projects purely out of some lofty principle and without technical merit. Heck, I'm struggling just trying to break my own habit of using blank? and present? everywhere. 😅 No doubt if those are removed in a future version of Bridgetown, some people will want to add them back in immediately for fear of going nuts!

With regards to this particular issue (adding a default Inflector to the config), this is a new feature for v2.0. Using something like dry-inflector shouldn't cause any major issues if we do it right, and that's something I'm quite comfortable with taking on. For example, Hanami seems to have dry-inflector working fine with Zeitwerk and it looks easy enough to set up acronyms like URL—see: https://github.com/hanami/hanami/blob/b73ad93534a2b5402d34dc365fe5fcbea52e8cbd/spec/integration/container/autoloader_spec.rb#L11

jaredcwhite commented 5 months ago

Jarrett makes a very good point. If you (@jaredcwhite) think DHH's removal of Typescript was a violation of the Code of Conduct, why are you doing the same thing?

@brandondrew Doing the same thing would be authoring a PR that completely removes all trace of Active Support etc. and then merging it in a few hours later. 😅 Thankfully we're not doing that!

jclusso commented 5 months ago

@jaredcwhite:

Please let me know which gems are owned by people violating their own CoCs and I will gladly take a close look. There's no slippery slope here!

I'm sure there's gotta be some. I'm not going down that rabbit hole though!

Vendoring is merely a near-term solution, I'll grant you that.

At least we can agree on this.

The long-term solution is to migrate off completely. This is certainly no easy feat and will take some real time and effort. For example, it only just dawned on me the other day that the CLI framework we use, Thor, is completely located within the Rails repo! It was actually migrated from I believe wycats (aka Yehuda Katz, the original author) some while back. I honestly have no idea at the present moment how to deal with that.

I want to start by saying I think your time is valuable, as well as that of anyone else that is contributing to OS. With that being said, it's disappointing to see that limited time spent replacing stuff, instead of on new stuff. You've written about some exciting things like Signals and what not. I'm sure most Bridgetown users would be way more excited to see a release with real changes vs. housekeeping.

Regarding Active Support specifically, it's obviously not going to blink out of existence right away. We'll whittle away at it a bit for v2, some more for v2.1, etc. And long-term, here's what I can promise you. If you want to require "active_support" in your own codebase and use any or all AR features, that's totally cool! Nobody here will bat an eye. And if doing so causes some weird compatibility glitch at some point, I'm absolutely in favor of filing issues and dealing with it.

Fair enough. I wasn't sure if the removal of AS would lead to situation like "we don't support AS, so that's not worth figuring out".

KonnorRogers commented 5 months ago

How is removing ActiveSupport moving this project forward? It's the 5th most popular gem and is extremely stable. I could understand the idea if it was to eliminate a dependency, or bring better functionality. This is just replacing one thing with another for no reason. This is quite a disappointment.

5th most popular gem just means its a dependency of Rails to be fair. I don't think that alone has any technical merit.

My personal view is ActiveSupport is a leaky abstraction that patches Ruby internals. Which can be a good thing in some cases, but its definitely a large monolithic gem as @ayushn21 pointed out.

ActiveSupport is also notorious for breaking conventions and needing funky workarounds to get bundler to work correctly.

I remember a few versions of Bridgetown hitting issues with ActiveSupport.

All that to say, it doesn't really matter to me what happens. But I also have very little love for ActiveSupport in the first place 🤷

jaredcwhite commented 5 months ago

After some testing, it looks like we can get all the functionality we need with Dry::Inflector, so I've just added that in #881. Can go ahead and close this one out.