Download & Extend
Issues

Unify all core package names to "Core"

Posted by Xano on October 9, 2008 at 11:20am
Project:Drupal core
Version:7.x-dev
Component:base system
Category:task
Priority:normal
Assigned:Xano
Status:closed (fixed)

Issue Summary

Since required Core modules aren't shown anymore displaying "Core - optional" is too much and it might be confusing (because where are the required modules then?). The attached patch changes the package from all optional Core modules from "Core - optional" to "Core". The required modules' package stays the same.

AttachmentSizeStatusTest resultOperations
core_package_00.patch15.35 KBIgnored: Check issue status.NoneNone
Login or register to post comments

Comments

#1

Posted by earnie on October 9, 2008 at 2:13pm

Maybe it should be package = Drupal instead? Or even package = Drupal core?

#2

Posted by Xano on October 9, 2008 at 2:34pm

+1 for package = Drupal core.

By the way, should core be capitalised or not?

#3

Posted by earnie on October 9, 2008 at 6:49pm

Based on other text, no.

#4

Posted by Xano on October 10, 2008 at 12:39pm

This patch changes "Core - optional" to "Drupal core" and additionally "Core - required" to "Drupal core (required)". I think using brackets is better for readability than a dash.

AttachmentSizeStatusTest resultOperations
core_package_01.patch21.33 KBIgnored: Check issue status.NoneNone

#5

Posted by earnie on October 10, 2008 at 1:14pm
Status:needs review» needs work

Looks nice but your system.module change is inserted in this patch file.

#6

Posted by Xano on October 10, 2008 at 1:31pm

Aaargh, I did a CVS update before creating the patch? :S

AttachmentSizeStatusTest resultOperations
core_package_02.patch18.54 KBIgnored: Check issue status.NoneNone

#7

Posted by earnie on October 10, 2008 at 2:39pm
Status:needs work» reviewed & tested by the community

Text change for package variable in info file is good.

#8

Posted by catch on October 10, 2008 at 3:48pm

I was going to rtbc this but earnie beat me to it.

#9

Posted by Dave Reid on October 10, 2008 at 3:55pm

This patch needed to change the string to hide the required group in admin/build/modules. Added the change to system.admin.inc from #6.

AttachmentSizeStatusTest resultOperations
core_package_03.patch19.09 KBIgnored: Check issue status.NoneNone

#10

Posted by earnie on October 10, 2008 at 4:23pm

Dave's addition is good. This is still RTBC.

#11

Posted by Xano on October 10, 2008 at 5:27pm

+1 for Dave's addition.

That package check could be done far better by using drupal_required_modules(), but that's something for another issue.

#12

Posted by Dave Reid on October 10, 2008 at 6:26pm

Separate but related issue in #319699: Simplify required module hiding in admin/build/modules.

#13

Posted by Xano on October 10, 2008 at 6:59pm
Status:reviewed & tested by the community» needs review

We're going to fix the problem Dave mentioned in #9 in #319699: Simplify required module hiding in admin/build/modules.

The attached patch puts all core modules in the same, human readable package.

AttachmentSizeStatusTest resultOperations
core_package_04.patch18.49 KBIgnored: Check issue status.NoneNone

#14

Posted by Dave Reid on October 10, 2008 at 7:10pm

Works and put all core modules into the same fieldset. Revised patch fixes the system module tests (fieldset used to enable/disable modules) and passes 100%!

AttachmentSizeStatusTest resultOperations
core_package_05.patch20.66 KBIgnored: Check issue status.NoneNone

#15

Posted by Dave Reid on October 10, 2008 at 7:54pm
Status:needs review» reviewed & tested by the community

Patch in #14 applies cleanly, passes all tests, and is RTBC.

#16

Posted by Xano on October 10, 2008 at 10:12pm
Title:Change package name: "Core - optional" -> "Core"» Change package name: "Core - optional" -> "Drupal core"

#17

Posted by webchick on October 11, 2008 at 12:12am
Status:reviewed & tested by the community» needs work

Shoot. Sorry guys, but we can't use "Drupal core" because of branded distros. http://drupal.org/node/81740#comment-443099 from the original package patch.

I agree that "Core - optional" is exceptionally silly now that Core required aren't shown. "Core" is fine, though. So could we get one that does "Core" and "Core (required)" ?

#18

Posted by Dave Reid on October 11, 2008 at 1:27am
Title:Change package name: "Core - optional" -> "Drupal core"» Unify all core package names to "Core"
Status:needs work» needs review

Revised patch that changes all core modules package info to "Core." There is no longer a "Core - required" because of the new required module implementation in #319699: Simplify required module hiding in admin/build/modules.

AttachmentSizeStatusTest resultOperations
core_package_6.patch20.43 KBIgnored: Check issue status.NoneNone

#19

Posted by webchick on October 11, 2008 at 1:40am
Status:needs review» needs work

system.test's hunk needs work.

#20

Posted by Dave Reid on October 11, 2008 at 1:52am
Status:needs work» needs review

Ah crap. Knew I shouldn't have done a grep for '= Drupal core'! System module test passes and confirmed the admin/build/modules screen appears as desired.

AttachmentSizeStatusTest resultOperations
core_package_7.patch20.41 KBIgnored: Check issue status.NoneNone

#21

Posted by webchick on October 11, 2008 at 2:33am
Status:needs review» fixed

Committed! :) Yay!

Now to look at that other eeeenteresting patch...

#22

Posted by Dave Reid on October 11, 2008 at 2:36am

Yay!

#23

Posted by Xano on October 11, 2008 at 5:40am

we can't use "Drupal core" because of branded distros

.

I thought forks etc. weren't supported? On top of that: it is the actual Drupal Core those packages belong to.

#24

Posted by Dave Reid on October 11, 2008 at 6:34am

I think webchick means branded Drupals such as Acquia Drupal, not forks.

#25

Posted by webchick on October 11, 2008 at 7:05am

Yeah. There are many ways to create a white-label distribution of Drupal that don't constitute "forking." For example, a university creating a distro of Drupal for all departments to create their own branded sub-sites. They would call the distro "Acme University Website Builder" not "Drupal." It's one of the reasons site_name is in a variable_get. We shouldn't hard-code the word "Drupal" in the interface.

#26

Posted by Xano on October 11, 2008 at 4:34pm

Isn't the name 'Drupal' used in a lot of strings too?

#27

Posted by webchick on October 11, 2008 at 5:09pm

Shouldn't be. If they are, they should be replaced with variable_get('site_name'). Separate issue, of course.

Can we let this one die now, so it quits bumping up in my "My issues" queue all the time? :)

#28

Posted by lilou on October 11, 2008 at 6:11pm

See this issue : #98905: Remove the word 'Drupal' from all user-facing stuff

#29

Posted by Anonymous (not verified) on October 25, 2008 at 6:12pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

nobody click here