field-package.0.png

Files: 
CommentFileSizeAuthor
#66 core-field-types-package-grouping-1255696-66.patch6.91 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 59,261 pass(es).
[ View ]
#66 interdiff.txt880 bytesswentel
#62 core-field-types-package-grouping-1255696-62.patch6.97 KBswentel
FAILED: [[SimpleTest]]: [MySQL] 59,423 pass(es), 12 fail(s), and 0 exception(s).
[ View ]
#58 field_type_package.png66.52 KBxjm
#57 core-field-types-package-grouping-1255696-57.patch7.48 KBlslinnet
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es).
[ View ]
#55 core-field-types-package-grouping-1255696-55.patch6.89 KBlslinnet
FAILED: [[SimpleTest]]: [MySQL] 58,110 pass(es), 8 fail(s), and 0 exception(s).
[ View ]
#51 core-field-types-package-grouping-1255696-51.patch3.88 KBlslinnet
FAILED: [[SimpleTest]]: [MySQL] 58,008 pass(es), 21 fail(s), and 0 exception(s).
[ View ]
#47 core-field-types-package-grouping-1255696-47.patch4.33 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] 56,607 pass(es), 21 fail(s), and 0 exception(s).
[ View ]
#43 core-field-types-package-grouping-1255696-43.patch4.3 KBjenlampton
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-field-types-package-grouping-1255696-43.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#37 module-packagitis.png41.34 KBsun
#34 1255696-34.patch10.22 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 47,993 pass(es).
[ View ]
#34 interdiff.txt1.27 KBdagmar
#27 field_types.png84.91 KBdagmar
#26 1255696-26.patch10.71 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 47,562 pass(es).
[ View ]
#26 interdiff.txt5.82 KBswentel
#17 drupal-framework.field-package.17.patch10.64 KBdagmar
PASSED: [[SimpleTest]]: [MySQL] 46,221 pass(es).
[ View ]
#17 interdiff.txt784 bytesdagmar
#15 drupal-framework.field-package.15.patch10.61 KBdagmar
FAILED: [[SimpleTest]]: [MySQL] 46,172 pass(es), 51 fail(s), and 9 exception(s).
[ View ]
#15 interdiff.txt6.56 KBdagmar
#13 drupal-framework.field-package.13.patch4.05 KBdagmar
FAILED: [[SimpleTest]]: [MySQL] 45,990 pass(es), 24 fail(s), and 1 exception(s).
[ View ]
#2 drupal-framework.field-package.2.patch3.57 KBsun
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-framework.field-package.2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
field-package.0.png29.68 KBsun
drupal-framework.field-package.0.patch2.54 KBsun
FAILED: [[SimpleTest]]: [MySQL] 33,431 pass(es), 19 fail(s), and 3 exception(es).
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, drupal-framework.field-package.0.patch, failed testing.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new3.57 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-framework.field-package.2.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Attached patch should fix the module dependency test failures.

tstoeckler’s picture

Patch looks good.
I like the idea, but probably needs some more people saying that before RTBC.

yched’s picture

I could go with this, but moving 'taxonomy' to the Fields package probably needs a broader approval.

jhedstrom’s picture

Moving taxonomy into the fields package seems non-intuitive, even though it's technically a field-based module now. Aside from that (and I'm not necessarily opposed), I'm all for moving the other field modules into their own package.

tstoeckler’s picture

I stumbled on Taxonomy as well, but then I realised, that while you can set up Vocabularies and Tags without having anything to do with fields, you can only do anything with those, if you attach them to something else, as a field. So I do think it makes sense.

sun’s picture

rickvug’s picture

I think this makes sense, even for less clear cut cases such as Taxonomy. We should privilege the needs of new and confused Drupal site builders who simply want to see all modules that provide fields.

swentel’s picture

Like the idea as well, but this will need an update probably because we now also have the email field, I'll trigger the re-test button but I have feeling it won't even pass anymore.

swentel’s picture

Status:Needs review» Needs work
Issue tags:+Usability, +Platform Initiative

The last submitted patch, drupal-framework.field-package.2.patch, failed testing.

yched’s picture

FWIW, I have no problem with this.

As a side note, though: even though the recent new field types (url, mail - soon date and entity_reference ?) have been added as submodules of /core/modules/field (next to the existing text, number, etc...), it would be good to get MAINTAINERS.txt entries for those, and not assume they are maintained by the Field API team :-), or we're going to hit the CCK effect (field modules get neglected because maintaining them all + the API itself is too big of a task).

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new4.05 KB
FAILED: [[SimpleTest]]: [MySQL] 45,990 pass(es), 24 fail(s), and 1 exception(s).
[ View ]

Re-rolled. Included email and link fields.

I think the MAINTAINERS.txt should go into a separated issue.

Status:Needs review» Needs work

The last submitted patch, drupal-framework.field-package.13.patch, failed testing.

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new6.56 KB
new10.61 KB
FAILED: [[SimpleTest]]: [MySQL] 46,172 pass(es), 51 fail(s), and 9 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, drupal-framework.field-package.15.patch, failed testing.

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new784 bytes
new10.64 KB
PASSED: [[SimpleTest]]: [MySQL] 46,221 pass(es).
[ View ]
tstoeckler’s picture

Status:Needs review» Reviewed & tested by the community

Looks good. I looked through the list of modules and this covers it.

sun’s picture

webchick’s picture

Assigned:sun» Bojhan
Status:Reviewed & tested by the community» Needs review

I see a lot of devs in here saying +1, but no UX reviews. Assigning to Bojhan.

Bojhan’s picture

Could this have a screenshot or list? E.g. Do we actually make fields part of this package too?

sun’s picture

@Bojhan:
The result is still the same as exposed in the screenshot in the OP. Only the new E-mail and Link modules are missing in there.

Bojhan’s picture

Why isn't Field, Field UI, Field SQL storage part of this group? It seems those are the container modules, all these modules are related to?

sun’s picture

The original/underlying idea is:

As a user, I want to have an easy/summarized access to toggle field functionality on my site.

This excludes Field and Field UI by design. As a user, I'm interested in what (actual) fields I can enable (or which I have enabled). Thus, mixing Field and Field UI into the list would inherently add off-topic clutter, and also open the door for arbitrary "field-related" modules in contrib to list themselves there, which would be wrong.

The concept only works for field modules, since almost all of them are sufficiently decoupled from the rest of the system and primarily/solely provide field functionality only.

Partially OT: In three to five years from now, it might also work for entity modules (i.e., node, comment, etc), in case they will have been reduced to pure entity behavior / business logic at that point. But that's not remotely a reality today. Field type modules differ, because they are at this point already; they do not provide anything else than field functionality. Thus, if there was any point in not enabling Field module on a Drupal site, then we'd actually want to improve the UX by hiding the entire package of field add-on modules if Field module is not enabled, since there'd be absolutely no point in exposing that clutter. Today, it's close to impossible to run a Drupal site without fields though, so that part is rather irrelevant, at least for field type modules.

Personally, I only have a very small nit to add to this proposal and current patch: I'd prefer "Fields" (plural) as label for 1) translatability, but also 2) clarification that all of these are "Fields", not "Field"-related.

Bojhan’s picture

Well the idea of grouping, is to add similar functionality together - that includes the "main" thing, since that is your orientation module. So I disagree it adds clutter. I agree that its probably never going to be clicked, so from that perspective I am fine keeping it in system.

However, the label "Fields" should be more descriptive, e.g. "Field types" - it is not clear this is about the actual types if we only use the label "Fields" then I have no doubt that field-related modules will start adding their stuff there.

FYI, I really like this - because it sets the trend of unwrangling the "core" package in more useful groupings.

swentel’s picture

StatusFileSize
new5.82 KB
new10.71 KB
PASSED: [[SimpleTest]]: [MySQL] 47,562 pass(es).
[ View ]

Changed to Field types, I personally like it in a way as well, although you could argue that it's maybe to technical ? Anyway here goes, I also attached the interdiff, although that's half as big as the patch itself :)

- edit : too much 'maybe's' in one sentence

dagmar’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new84.91 KB

Lets gets this in

field_types.png

catch’s picture

I'm not sure about Taxonomy - what if we wanted to add an Entity package later on?

xjm’s picture

Yeah, IMO taxonomy should not be in the group. It provides a field, but it itself isn't a field. File I'm not sure about.

Edit: on the other hand I'm not sure an entity package would mean anything to users. But taxonomy is more like comment than anything else in that list.

Edit 2: We could move the term reference field into its own module, or it could just be part of the entity reference field module once that's added... ;)

xjm’s picture

Also, it seems a little odd to have only one subset of modules that are not under "Core."

Bojhan’s picture

Assigned:Bojhan» Unassigned
webchick’s picture

Status:Reviewed & tested by the community» Needs work

Needs a re-roll for #28 and #29, looks like.

xjm’s picture

Issue tags:+Novice

Nice easy reroll. I'd suggest removing taxonomy from the group for now and opening a revisit-before-release type followup to discuss it further when we have a better idea of what other packages we should consider.

I also pinged @davereid about this issue since he's had concerns about the use of package for sorting modules in contrib in the past.

dagmar’s picture

Status:Needs work» Needs review
StatusFileSize
new1.27 KB
new10.22 KB
PASSED: [[SimpleTest]]: [MySQL] 47,993 pass(es).
[ View ]

Removed taxonomy from the Field types package.

sun’s picture

Status:Needs review» Closed (won't fix)

Recent comments indicate that this will not be solved in a satisfying way and the result will only confuse users.

Sorry for wasting everyone's time.

The proper answer to this is #623544: How to categorize a module as developer module, which I once tried to push forward via #624848: Allow to retrieve a list of modules defining a certain .info file property as a first step, but that got blocked on the D7 API freeze. It's probably time to re-start that effort.

yoroy’s picture

Status:Closed (won't fix)» Needs review

How does a "what if" and a "little odd" warrant a won't fix? I like how this idea groups all the modules that make stuff show up in Field UI, a little almost subliminal connecting of the dots. Having Taxonomy in here might actually help some people see that exposing tags happens as a field on your content type for example. I don't understand what #623544: How to categorize a module as developer module is about.

sun’s picture

StatusFileSize
new41.34 KB

The actual problem we have is this:

module-packagitis.png

(Note: The page didn't fit on my screen [not even two], so I had to hack the CSS to get this in a shot.)

#623544: How to categorize a module as developer module and related issues ultimately want to solve this by ditching "package" altogether.

Instead of "package", we introduce tags. Meaning, a module can be classified into multiple categories. At the same time, the total set of all categories makes more sense, because we stop the "packagitis" problem that is being caused by tons of modules creating their own, new package, because they think they are special (they are not).

Instead of a single package, there will be multiple tags/categories, meaning that modules like Taxonomy can easily appear in both "entity" and "field". — In which of both would you search it? Doesn't matter, it's there!

Lastly, to avoid tagitis/categoritis, we automatically strip away all tags that only appear once. Thus, if only one module tags itself as "beavis", and only one other module tags itself as "butthead", then neither of both categories will appear. Each of both modules will have tagged itself with some other terms and can be found within those (hopefully more reasonable) categories.

xjm’s picture

I would agree with classifying this as a duplicate of a core issue for #623544: How to categorize a module as developer module; I was a little ishy on the whole direction of this patch and that seems like the better solution.

Edit: didn't realize the issue was in a different queue. We'd need a core issue for .info tags. I think @davereid has a module for that somewhere.

sun’s picture

Gábor Hojtsy’s picture

#1833184: Find a consistent naming scheme for translation-related modules is leaning towards establishing a "Language" or "Multilingual" category in core that would let contrib modules add more modules in that category. Now that there are three interacting modules in core to deal with language-related stuff. There however the thinking was to (a) include all core modules related to language (b) let contribs add more multilingual modules there. This is somewhat different from the goals that this issue had to only group a subset of field modules. However it would set up another group that is not "Core" in core. Just to point out there is likely a need for this categorisation to sub-divide core with the explosion of functionality (and in some cases better functionality separation) we have in Drupal 8.

sun’s picture

sun’s picture

Status:Needs review» Closed (duplicate)
jenlampton’s picture

Status:Closed (duplicate)» Needs review
StatusFileSize
new4.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-field-types-package-grouping-1255696-43.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Re openeing since we should have field-type modules grouped separately on the modules page, even if #1868444: Introduce tags[] in module.yml file to categorize modules by provided functionality. does not land before d8.

Also, patch attached.

Status:Needs review» Needs work
Issue tags:-Usability, -Novice, -Platform Initiative

The last submitted patch, core-field-types-package-grouping-1255696-43.patch, failed testing.

jenlampton’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+Usability, +Novice, +Platform Initiative

The last submitted patch, core-field-types-package-grouping-1255696-43.patch, failed testing.

jenlampton’s picture

Status:Needs work» Needs review
StatusFileSize
new4.33 KB
FAILED: [[SimpleTest]]: [MySQL] 56,607 pass(es), 21 fail(s), and 0 exception(s).
[ View ]

needed reroll

Status:Needs review» Needs work

The last submitted patch, core-field-types-package-grouping-1255696-47.patch, failed testing.

tstoeckler’s picture

picture.module does not provide a field type, it simply provides a formatter. So if we call the package "Field types" then Picture should not go in there.

lslinnet’s picture

Status:Needs work» Needs review
lslinnet’s picture

StatusFileSize
new3.88 KB
FAILED: [[SimpleTest]]: [MySQL] 58,008 pass(es), 21 fail(s), and 0 exception(s).
[ View ]

@tstoeckler you are quite right, have rerolled a new patch which excludes the picture module.

Status:Needs review» Needs work

The last submitted patch, core-field-types-package-grouping-1255696-51.patch, failed testing.

lslinnet’s picture

I have started fixing the test cases related to this issue.

tstoeckler’s picture

Title:Move field type modules into separate Field package» Move field type modules into separate "Field type" package

Fixing title to be more accurate in that case.

lslinnet’s picture

StatusFileSize
new6.89 KB
FAILED: [[SimpleTest]]: [MySQL] 58,110 pass(es), 8 fail(s), and 0 exception(s).
[ View ]

Have updated the test cases which failed, was unable to run the locale test case on my local environment due to reaching max nesting level in php.

lslinnet’s picture

Assigned:Unassigned» lslinnet

Just assigning it to myself

lslinnet’s picture

Status:Needs work» Needs review
StatusFileSize
new7.48 KB
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es).
[ View ]

Missed one reference to image module in the LocaleConfigTranslationTest, should be fixed in the latest patch.

xjm’s picture

StatusFileSize
new66.52 KB

So I'm still kind of ambivalent (/cc @alexpott) about this.

On the one hand, the "we shouldn't use package for module tagging/categorization" statement really doesn't work now that multilingual is using its own package anyway. And this probably does improve usability (versus having to search through the whole list).

However, I'm kind of unsure still why we don't just fix #1868444: Introduce tags[] in module.yml file to categorize modules by provided functionality.. That change would be an API addition, not a BC break, so it can go in right up to RC1. (It's feature-y, so maintainers might choose to hold the patch until our major/critical counts are better, but the issue isn't RTBC anyway.) The solution in this issue seems too simplistic to me, and here's why:

  • @lslinnet has added only actual fielt type modules to the list, which makes sense.
  • Conceptually, field types themselves are plugins and not modules. A module should not need to be exclusively about the field type it provides. A module should be able to provide one or more field types along with other functionality. And, indeed, this is already the case with taxonomy. (If we manage to get #1847596: Remove Taxonomy term reference field in favor of Entity reference in that will change, but it's still a completely legitimate usecase for contrib.)
  • If I were a site builder, I'd probably be confused as to why taxonomy wasn't in the list. I'd also be confused when a contributed module that provided a field type along with other functionality didn't show up in the list. I might even be confused as to why other field-related modules (that provide widgets, formatters, and the like) didn't show up in the list.
  • In short, tags make a bunch more sense than package for categorizing field modules.

Codewise, the patch is fine. Here's what it looks like in the UI.
field_type_package.png

I also verified that these are the only exclusive field type modules in the list.

jenlampton’s picture

Status:Needs review» Reviewed & tested by the community

The problem with waiting for tags is that we haven't figured out a usable UI for them yet, or a way to make the infinite list of tag possibilities manageable. But, packages, we have now :)

In short, I think we should do both things. I think we should do this - now - because it will make things better - immediately.
*And* I think we should add tags, once we figure out all the details and resolve all the associated problems that go along with them.

Patch still applies cleanly. :)

amateescu’s picture

Status:Reviewed & tested by the community» Postponed

There's a discussion going on in #2064727: Separate Core modules into separate packages on module page about how to do this generally for all core modules and I think we should wait a bit for its outcome..

johnv’s picture

As a site builder and parttime coder, this new package is a huge help. My two cents:
- This discussion is very old for a clear UX improvement. Just get it in. It splits off a 30% of the core modules.
- The beste name should be 'Field', since the list contains modules leveraging the Field API. (The 'Options' module does not provide a Field type, only widgets+formatters.)
- The proposal in #2064727 leans towards 'Core Field'. We shouldn't do that. Contrib modules are welcome in the 'Field' package.
- About Taxonomy: I would add it to the Field package, but it is not worth delaying this improvement.
- Tags are fine, I guess. But normally I just scan the modules list using a text search. I'm happy with the Package-way.

BTW. This is another approach: #1371524: Remove packages from modules list and .info files

swentel’s picture

Status:Postponed» Needs review
StatusFileSize
new6.97 KB
FAILED: [[SimpleTest]]: [MySQL] 59,423 pass(es), 12 fail(s), and 0 exception(s).
[ View ]

This now also got more interesting now that comment is a field ... :)

Just a reroll. Web services just go into a separate package too, so why don't we just make a decision here and go ahead with this ?

Status:Needs review» Needs work

The last submitted patch, core-field-types-package-grouping-1255696-62.patch, failed testing.

amateescu’s picture

Because we don't have an agreement yet in the other issue.. ?

Bojhan’s picture

I think we can just move ahead.

swentel’s picture

Status:Needs work» Needs review
StatusFileSize
new880 bytes
new6.91 KB
PASSED: [[SimpleTest]]: [MySQL] 59,261 pass(es).
[ View ]
Bojhan’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community
webchick’s picture

Priority:Normal» Major
Status:Reviewed & tested by the community» Active
Issue tags:+Needs Documentation, +Needs change record

Ok, seems we have consensus here.

I was initially confused by why e.g. Taxonomy, Image, etc. don't show up here but it looks like this was covered above. Basically, if the module's doing other things and also defining a field, it goes in whatever package makes the most sense. If it only defines a field, it goes here. Feels like we need to capture that somewhere in e.g. the UX guidelines so that module developers know what they're supposed to do here.

So tagging for both a change notice and documentation.

Committed and pushed to 8.x. Thanks!

webchick’s picture

Title:Move field type modules into separate "Field type" package» Needs change notice/docs: Move field type modules into separate "Field type" package
Bojhan’s picture

Issue tags:+Needs UX Guideline

I am making up a new tag here.

tstoeckler’s picture

Re #68: Well in fact Image is in there but taxonomy is not, so this is now *hugely* inconsistent in core. :-/

Bojhan’s picture

You define huge, as 1? :P

webchick’s picture

It's a fair point. Why is taxonomy special? We need to be able to very clearly and simply define this, or else we need to roll this patch back.

Berdir’s picture

The current separation makes sense to me. The main thing that image.module does is providing image fields. It also provides image effects and stuff like that, but in most cases, they're used in combination with image fields. You install image.module because you want to add image fields.

taxonomy.module is about categorizing/structuring your content, it just uses the field system to store the assignments. That would be even more obvious if it would use the entity reference field type and not provide it's own. Comment is similar.

Les Lim’s picture

This doesn't represent an API change; does it still need a change notice?

Les Lim’s picture

Also, for some reason, I get "Access denied" when I try to update/edit this issue. I can access the edit page for other core issues just fine.

klonos’s picture

@Les Lim: happened to me in another issue. It's because some user that had the proper rights edited the issue summary in a text format that allows more html tags than what we regular users are allowed to. So, now that we try to edit we have no permissions.

tim.plunkett’s picture

Issue summary:View changes

Fixed input format

xjm’s picture

Title:Needs change notice/docs: Move field type modules into separate "Field type" package» Needs docs update: Move field type modules into separate "Field type" package
Priority:Major» Normal
Issue tags:-Needs change record

I don't actually think this needs a change record. Also, as far as I know, the "use package only for actual honest-to-goodness packages" recommendation was mostly unwritten and proclaimed in issues, but if not, then we still need a docs update.

Dave Reid’s picture

We probably *should* have a change record because all the field types in D7 standardized on 'Fields' as the package and will have to change this line in D8 to match what core is doing. Insert irony here about how core went and did something different than a well-established pattern in contrib.

Gábor Hojtsy’s picture

@Dave: I think I captured that in a change notice that I just added in https://drupal.org/node/2186029. Not sure where this needs a docs update though.

plach’s picture

@Dave: @Gabor:

If I read that CN correctly, it means the the Fieldgroup module should not go in the "Field type" package, but in a more proper "Fields" (or similar :) package, right? Sorry if this has already been answered but I didn't follow the discussion here.

mgifford’s picture

Assigned:lslinnet» Unassigned
connorwk’s picture

Issue tags:-Novice

Triaging as not Novice because it looks like it went into a discussion giving no clear path forward.