Naming of katello_content_view_version_promote

[gregswift]

Looking through the modules (thank you!) and I'm curious. katello_publish is only for content views, but doesn't have content view or version in the name. Then katello_sync isn't katello_repository_sync. Why not rename the promote module from katello_content_view_version_promote_ to katello_promote?

just a thought

[mdellweg]

@gregswift thank you for sharing your opinion. I remember, we had a related discussion as the name was katello_contentview_promote. There was a lot of confusion around what it was doing. I personally prefer the names that state what action is performed on which entity. So maybe we should rename katello_publish to katello_contentview_publish and so on.

[sean797]

+1 to katello_content_view_publish from me. That underscore is needed to match with the other modules :-)

[akofink]

I personally prefer the names that state what action is performed on which entity.

Yes, me too. PRs welcome! :wink:

[gregswift]

Okay... so that makes me sad but I can live with it.

Would it then make sense that katello_content_view_publish should actually be katello_content_view_publish_version so that you can take the action names and understand how the separate action relate? Otherwise why have version in the katello_content_view_version_promote ?

[mdellweg]

I can imagine, that you started the thread to shorten one name and now we are discussing about prolonging two (or more?). But we had the discussion about katello_content_view_version_promote: https://github.com/theforeman/foreman-ansible-modules/pull/38#issuecomment-325652506 And for the publish: It is the 'content_view' that gets 'published', resulting in a new version. Calling the module katello_content_view_publish_new_version doesn't improve the situation IHMO.

[gregswift]

While shortening was my preference my goal was consistency.

The reason why i suggested publish version is because that is the action in my mind. You are publishing a new version, so yes, your longer one is more accurate.

If the intent of having the 'content_view' in the name is to make it explicit of what the action is applying to, even though its the only thing that action can apply to.

I'm still not sure that's coming across clearly. From a consistency standpoint it seems that the goal of the naming is:

katello[_<entity>][_<action>]

Rules:

• Config management related modules are primarily just katello_<entity>.
• For orchestration type actions <entity> is optional.

So now we are at the point of what should be inside the action. Orchestration actions that exist right now:

• katello_content_view_version_promote Tells you explicitly that it is promoting <action> a version <subentity> of a CV <entity> Which translates to katello_<entity>_<subentity>_<action>
• katello_publish publishes <action> something...which if you know Katello means a version <subentity> of a CV <entity> Which translates to katello_<action> ** But if we change it to katello_content_view_publish it becomes katello_<entity>_<action>
• katello_sync syncs <action> something. if you know Katello would just mean a repository <entity>, but this actually will sync all repos in a product <entity>. i just found that in the ui.. didnt even know it was a thing.. and was definitely something i had to dig for. Which translates to katello_<action>

Now... i get that since sync has multiple entities, it makes it harder to attach it to something.. so that can make sense.

am i over thinking ?

[mdellweg]

❤️ consistency! Surely no overthinking here. I am fine with the (foreman|katello)_<entity> scheme for creation, deletion, modification and (foreman|katello)_<entity>_<action> scheme for the action modules. I would not call the cv_version a subentity, however.

[ehelms]

I like this overall discussion to ensure we land on consistent and obvious names for users. This would mean that for syncing, we'd split this across two modules?

katello_product_sync katello_repository_sync

I am find with that, since we have code sharing which helps prevent a lot of the re-writing of code and these do become more obvious actions.

[rackergs]

I would not call the cv_version a subentity, however.

so what is a version then?

[mdellweg]

I see the _content_viewversion as the entity. Like the reopository ist an entity, even if it can only live attached to a product.

[gregswift]

fair enough... but is a version called content_view_version anywhere? i mean a repository isnt called product_repository (which would almost be nice sometimes)

[akofink]

is a version called content_view_version anywhere?

https://github.com/Katello/katello/blob/master/app/models/katello/content_view_version.rb

Or do you mean within foreman-ansible-modules?

[gregswift]

you win :D (and i mean that with light hearted agreement.. not derision)

[mdellweg]

@ehelms I think, this issue has been closed prematurely. There is the split of katello_sync open still. Should it be a new issue? Has it been decided? Feels like that.

[ehelms]

I'd recommend a new issue given the title of this issue would be misleading.

[mdellweg]

I will make one.