backdrop-contrib / paragraphs

Paragraphs module to control your content flow
https://backdropcms.org/project/paragraphs
GNU General Public License v2.0
5 stars 11 forks source link

[2.0] Rename `bundle` to `type` and deprecate functions/permissions that use `bundle`. #23

Open laryn opened 6 years ago

laryn commented 6 years ago

EDIT/CLEAN UP:

The term "bundle" has been changed in front-facing places but it remains to clean up the code (function names, permission names, etc.) and deprecate the old "bundle" versions in the same way Backdrop core:

laryn commented 5 years ago

Work in progress locally (will push this week hopefully) but noting here that the label/machine name column(s) should track whatever is decided here, probably (for consistency):

https://github.com/backdrop/backdrop-issues/issues/3395

laryn commented 5 years ago

Just pushed these commits in a new branch for testing. @herbdool do you use Paragraphs? Would you be willing to help me test?

screenshot from 2018-11-29 09-30-44

herbdool commented 5 years ago

@laryn sadly I'm not using Paragraphs. If you can't find anyone to help test I'll try to find some time in the near future.

serundeputy commented 5 years ago

@laryn In general I'm for this idea as it brings them in parallel with nodes and entities that have types. :+1:

The current PR seems to be addressing the UI references to bundles in favour of type :+1:

I think this might lead to some less than ideal Developer Experiences [DX] though; :-1:

For example the function paragraphs_bundle_load should probably be renamed to paragraphs_type_load to be brought semantically inline with (what I think is the spirit of this issue), but that would be an API change, so folks coming from D7 paragraphs could have potentially breaking change here.

That leaves a couple of options here;

Not sure what is the best way to go ???? the third option is probably best, but also the hardest.

klonos commented 5 years ago

provide a wrapper where paragraphs_bundle_load calls call paragraphs_type_load and mark paragraphs_bundle_load as deprecated

^^ that's what we've been doing in core. I think that this is the best option.

laryn commented 5 years ago

@serundeputy @klonos I can probably be convinced. I was leaving it as is (just changing the UI) for the moment in the hopes that any potential future updates from the D7 module would apply more cleanly. Maybe I don't need to worry about that.

robertgarrigos commented 5 years ago

I was trying this issue-23 branch and got an error when creating a new paragraph type:

Warning: Creating default object from empty value a paragraphs_admin_bundle_form_submit() (línia 225 de /app/www/modules/contrib/paragraphs/paragraphs.admin.inc).
laryn commented 5 years ago

Status: I think I'm going to leave Backdrop Paragraphs 1.x-1.x more or less as it is on Drupal 7 and begin these changes in Paragraphs 1.x-2.x. (I don't envision D7 Paragraphs getting a lot of attention and massive changes, but these updates will make a lot of changes to the Backdrop version.) Also planning to follow the third option listed by @serundeputy and thumbed up by @klonos.