huacnlee / rails-settings-cached

Global settings for your Rails application.
Other
1.06k stars 202 forks source link

Prevent name collisions with ledermann/rails-settings #252

Closed Roy-Gal-Git closed 1 month ago

Roy-Gal-Git commented 1 month ago
  1. Fix - Changed the main module name from RailsSettings to RailsAppSettings since ledermann/rails-settings uses the same module name.
  2. Fix - Changed the generator namespace from settings to app_settings to prevent name collisions with ledermann/rails-settings since it uses the same generator name.
  3. Feature - Setting a custom model name in the generator (rails g app_settings:install AppConfig will create a migration with the matching database name.
huacnlee commented 1 month ago

Obviously this change is unacceptable:

  1. It is impossible to change the gem name.
  2. There are already a large number of users using it, so changing the module name will cause a lot of breaking changes, but compatibility with rails-settings is not a must, instead I think you should choose to remove rails-settings.
huacnlee commented 1 month ago

Sorry not to accept of your changes. Again I said reason of this Gem not target to compatibility to rails-settings, it a replacement. If not this I was just need to make pull request to rails-settings.

huacnlee commented 1 month ago

Or If you real want use them both, I suggest you to dependency your's fork.

But even this I suggest you make a change to move your old rails-setitngs to rails-settings-cached. Compared with to make those changes, maybe you don't need to too much works.

Roy-Gal-Git commented 1 month ago

@huacnlee How come this change is unacceptable?

It is impossible to change the gem name.

I didn't change the gem name, I changed the module name.

There are already a large number of users using it, so changing the module name will cause a lot of breaking changes, but compatibility with rails-settings is not a must, instead I think you should choose to remove rails-settings.

Your gem is not a replacement for rails-settings. rails-settings and rails-settings-cached do 2 different things. rails-settings manages scoped settings (i.e. for each record) while rails-settings-cached manages global settings. Users can't migrate from rails-settings to rails-settings-cached simply because you don't have a scoped settings feature implemented in your gem. For that reason - breaking changes are a necessity to allow users to use both gems.

While I'll totally understand if you don't want any new breaking changes in your gem, changing the module name to anything that's not RailsSettings is a thing that you should've done since you dropped support for scoped settings.

Please reconsider merging this PR - you'll only benefit from more users being able to use your gem.

huacnlee commented 1 month ago

To rename RailsSettings to RailsAppSettings is a large change, this need all exist user to must update there code. So I don't want do this.

To compatibility with rails-settings is not the target or must.

In your system that must have one gem for one feature. You said the scoped settings, I think you can rewrite them (Actually my project was done this).

Here is a ref maybe can help you:

https://github.com/ruby-china/homeland/blob/01e8079b16ab7ce19a305865c9c4fe0960d31556/app/models/concerns/scoped_setting.rb

https://github.com/ruby-china/homeland/commit/6dd4dc293cfb7818631e36d9ad41bf83bc4ea4dc

Roy-Gal-Git commented 1 month ago

@huacnlee I'll stick to a fork, thanks for your help!

Roy-Gal-Git commented 1 month ago

I suggest you change the MIT-LICENSE to match the format in this PR to avoid legal issues. Source: https://opensource.stackexchange.com/a/8854/35395

Roy-Gal-Git commented 1 month ago

Hi @huacnlee,

Thank you for your response and for taking the time to explain your perspective. I understand the concern regarding breaking changes, and I certainly appreciate the value of maintaining backward compatibility for existing users.

However, I would like to clarify a few points and offer a potential compromise:

1. Different Use Cases for RailsSettings vs. RailsSettings-Cached:

As mentioned earlier, rails-settings and rails-settings-cached are not direct replacements for each other. The two gems serve different purposes:

This means that users who rely on scoped settings can't simply migrate to rails-settings-cached, which doesn't offer the same functionality. They need both gems if they require both scoped and global settings in their projects. Without changing the module name, users are forced to choose one or the other, which limits the flexibility of the gem ecosystem.

2. Aliasing as a Potential Solution:

I completely understand that changing the module name would cause breaking changes for existing users, and that is certainly not ideal. As a compromise, what if we implemented module aliasing? This would allow for backward compatibility by keeping the RailsSettings namespace intact for users who rely on it, while providing a way for users who need both gems to avoid collisions.

However, there is an important consideration: if we alias RailsAppSettings to RailsSettings before rails-settings is loaded, it would cause issues since the rails-settings gem depends entirely on the RailsSettings module.

Therefore, we would need to ensure that rails-settings is loaded before applying the alias, to avoid overwriting its functionality. The alias should only happen if rails-settings hasn’t already been loaded. This brings us to the importance of controlling load order.

3. Load Order Consideration:

To safely apply the alias without breaking the functionality of rails-settings, we can control the load order of the gems in the Gemfile. By listing rails-settings first and rails-settings-cached second, we ensure that the alias only applies if rails-settings is not already defined:

gem 'rails-settings'
gem 'rails-settings-cached'

This ensures that rails-settings is fully loaded before the aliasing happens. In this way, if rails-settings is loaded, the alias won’t take effect, preserving its functionality. If only rails-settings-cached is loaded, the alias will map RailsAppSettings to RailsSettings to prevent conflicts.

4. Benefits of Collaboration:

By addressing this compatibility issue, we open the door to a broader audience of users who can benefit from both gems working in tandem. This would also reduce potential conflicts in the gem ecosystem, making both gems more versatile for the community.

I fully respect your decision if you prefer not to make any changes to the module name, but I believe this solution could strike a balance between maintaining backward compatibility and increasing the flexibility and reach of rails-settings-cached. I’d love to hear your thoughts on this approach.

Thank you again for your time and consideration!

huacnlee commented 1 month ago

RailsSettings handles scoped settings (per-record settings).

I don't agree this, the scoped settings is useless, that can be replaced by Rails serialize:

https://api.rubyonrails.org/classes/ActiveRecord/AttributeMethods/Serialization/ClassMethods.html#method-i-serialize

That is why I removed the scoped settings in this gem.

By this reason, I don't think that is a good idea, so please understand that I don't care to compatible with it. Because it just wastes time, we should use the feature that Rails provided to us.

I have give you a solution how to continue use your existing scoped settings data in your project, please check my previous comment again. That is also I did in real world project.

You can try it, if you meet any trouble, I'd like to help you :)

Roy-Gal-Git commented 1 month ago

Thank you for the response and for explaining your perspective. I understand your reasoning for using Rails' serialize to manage scoped settings, and I appreciate the solution you've offered. However, I’d like to clarify why I (and potentially others) still see value in the approach rails-settings takes with scoped settings.

1. Scoped Settings Use Case:

While Rails' serialize is certainly a powerful feature, it doesn’t fully cover the specific use cases that rails-settings addresses, such as managing settings for individual records in a flexible and more queryable way. RailsSettings allows for more than just storing serialized data—it offers additional functionality like default values, inheritance, and querying settings, which can be challenging to implement with just serialization.

For applications that need to manage per-user or per-record preferences and settings (with features like fallbacks or custom querying), RailsSettings provides a robust, structured solution that many users rely on. This functionality is not easily replaced by serialization alone without additional work to replicate the behavior.

2. Collaboration and Ecosystem:

My suggestion for changing the module and generator names was to allow both gems to coexist in the ecosystem, as I believe they serve slightly different purposes. Many users, like myself, would benefit from being able to use both gems together, especially in projects that require both global and scoped settings.

I fully understand and respect your decision to focus on a simpler approach for your gem, and I appreciate your offer to help with adapting serialize for existing projects. My goal was to make it possible for users to use both rails-settings and rails-settings-cached without conflicts, but I understand if that's not something you're interested in supporting at this time.

Thanks again for your time and for the helpful discussion.