[WIP] Registering passing a feature model

[pepicrft]

What?

I started using the Framework and I noticed that the way features are registered is calling the method passing all the parameters and internally (after some checks) the feature is registered and the model HUBFeatureRegistration created. I have a few ideas that I'd like to discuss with you first and get your point of view regarding them:

• The name HUBFeatureRegistration sounds like it's represents a registration in the registry. However it doesn't contain any registration information. How does it sound renaming it to just HUBFeature?
• Then the HUBFeatureRegistry could include a method for registering these features:

- (void)register:(HUBFeature *)feature;

I've only added the extra method but I'll wait for your answer before continuing with these ideas.

[JohnSundell]

Awesome to see your first contribution @pepibumur! 😉

Interesting idea! How would HUBFeature work, is it like a mutable configuration object, or do you set it up once with an initializer? (Agree that the name HUBFeatureRegistration sounds more like a token than an actual model object).

[spotify-ci-bot]

	1 Warning
⚠️	PR is classed as Work in Progress

Generated by 🚫 danger

[codecov-io]

Current coverage is 92.61% (diff: 0.00%)

Merging #102 into master will decrease coverage by 0.20%

@@             master       #102   diff @@
==========================================
  Files            58         58          
  Lines          4038       4047     +9   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           3748       3748          
- Misses          290        299     +9   
  Partials          0          0          

Powered by Codecov. Last update fbf5a8f...8c1de61

[pepicrft]

Interesting idea! How would HUBFeature work, is it like a mutable configuration object, or do you set it up once with an initializer? (Agree that the name HUBFeatureRegistration sounds more like a token than an actual model object).

@JohnSundell I wouldn't make it mutable, just instantiate it and pass it to the registry instead of calling the methods with all the parameters. Why not passing directly the model if you use it internally?. Regarding the name, that's the point, it sounds that it represents a feature + the registered state, and that's not the case. What do you think?

[JohnSundell]

@pepibumur Hmmm what would be the benefit of creating the object and then passing it to a method vs just calling the method? Wouldn't that turn something which can be done in one line to something a bit more complex? I'm not opposed, just want to figure out if it will have some clear API benefits.

Regarding the renaming of HUBFeatureRegistration into HUBFeature - all for that 👍 (although it's an internal object only - but always nice to have a clean house 😃)

[pepicrft]

@JohnSundell I think it simplifies testing, you could have factories that provide you with the features that your app/apps need:

• You could test if the factories return the features that you expect.
• You could test if the registry gets the features from the factory.
  Right now it's hard to test the registration process because the registration component imperatively calls the selector (in the example app you do it in the AppDelegate). What do you think?
  Regarding the naming, do you think I can just rename it, or should I provide any migration component/macro?

[JohnSundell]

@pepibumur Alright. That's a good point! So sounds like a good change 👍 Regarding naming, feel free to just replace the current registration method with registerFeature:, and create a HUBFeature class as part of the API. No need to use macros or migration, just full steam ahead! 🚀

[JohnSundell]

Feel free to also use the demo app as a demonstration of this new API, think what you said about creating a FeatureFactory would be an awesome showcase of why this change is great 👍

[pepicrft]

Awesome, I'll work on it @JohnSundell . Hub it up 🎉 🎉 🍷

[JohnSundell]

@pepibumur Wohooo! I'll try to get new t-shirts ordered ASAP! 🎉 🚀

[pepicrft]

Sorry @JohnSundell I haven't had time this week to work on it. I'll try to get it ready for this one.

[JohnSundell]

@pepibumur No need to be sorry, take your time 😄 It's not a part of the code base that sees a lot of movement, so shouldn't be any problems to implement it whenever you have time 😄