TEDx installation profile is an installation profile for TEDx websites. It contains contenttypes, views and blocks. Also some theme functions have been added for customizing the header.

  • 4 Contenttypes: page, news, speakers & video set up en ready to go
  • Views for the different contenttypes
  • Superfish navigation
  • Easy placement of page content on the frontpage
  • Easy image and Vimeo video selection in the header

New version available (including .make file) please review.
array_merge(): Argument #2 is not an array install.inc:1165 is caused by the latest Superfish module. Choosing Superfish 1.9-beta4 solves this error. And will be included when 1.9 is released.

Sandbox page

Drupal version: 7.x

CommentFileSizeAuthor
#20 drupalcs-result.txt175.94 KBklausi
#2 output.txt92.5 KBwesleydv

Comments

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

The response time for a review is now approaching 4 weeks. Get a review bonus and we will come back to your application sooner.

wesleydv’s picture

wesleydv commented
Status: Needs review » Needs work
StatusFileSize
new output.txt92.5 KB
wesleydv’s picture

wesleydv commented
  1. An install profile should be an entire Drupal install folder with the install profile inside the profiles folder.
  2. It should also include all the depended modules so it can be installed without any further downloads needed. A good example of install profile is http://drupal.org/project/commerce_kickstart
  3. On installing the profile i received the following error: array_merge(): Argument #2 is not an array install.inc:1165
  4. I think you should enable the toolbar module (or an other equivalent) because it will be quite difficult to navigate to the the admin page for new Drupal users.
  5. Review the automated error report in the previous comment
betawerk’s picture

betawerk commented

Thank you for all the replies.
We were not aware of the fact that a installation profile should be a complete Drupal install folder. Thanks for clearing this up!
We will be updating the profile tomorrow.

wulff’s picture

wulff commented

Re: the first point in #3: This is not entirely true, AFAIK.

An install profile project needs to contain the project-specific modules and themes as well as a .make file which the d.o packaging script can use to build the distribution (i.e. pull in the necessary contrib modules and third party libraries). See http://drupal.org/node/642116 for more information on how to package a distribution (and take a look at a project like COD for a working example: http://drupalcode.org/project/cod.git/tree/refs/heads/7.x-1.x).

A few minor nitpicks:

  • You should use a .make file to pull in the external libraries instead of including them in the repository.
  • You should probably get rid of the .DS_Store file in the repository and add it to your .gitignore.
betawerk’s picture

betawerk commented

Thanks again for the comments. So if I'm right, I can just create a .make file with all the needed versions of modules in it. (including the latest release of Drupal). Do I need to use Drush Make or can i just type a .make file?

betawerk’s picture

betawerk commented

The profile is updated. The included .make file generates the Drupal folder and downloads all necessary modules. We are looking for reviewers so we can share the project to the public!

betawerk’s picture

betawerk commented
Status: Needs work » Needs review

Project is updated and ready to go. All review are welcome!

Auke Rick’s picture

Auke Rick commented
Status: Needs review » Needs work

Please check: http://ventral.org/pareview/httpgitdrupalorgsandboxbetawerk1416280git

betawerk’s picture

betawerk commented
Status: Needs work » Needs review

Lot's of the errors are caused by the colorbox library, so i included the library within the .make file. New version pushed!

betawerk’s picture

betawerk commented
Issue summary: View changes

Nieuwe version uploaded to Sandbox page.

patrickd’s picture

patrickd commented
Status: Needs review » Needs work

Still lots of major issues left (master branch, readme missing, ...)
Avoid putting 3rd party code or modules directly into your profile.

Please only switch back to needs review when the majority of errors are solved
You can use ventral.org/pareview to recheck yourself, if you got any questions on that please ask.

betawerk’s picture

betawerk commented

We have added the readme.txt, changed a lot of code and added some extra gitignore files. Only one problem when we are trying to create a branch or even upload in the master we receive the following error. 

error: Cannot access URL http://git.drupal.org/sandbox/betawerk/1416280.git/, return code 22
fatal: git-http-push failed

How is this possible?

betawerk’s picture

betawerk commented

And suddenly after twenty tries it works again. Created branch 7.x-1.x.
Still receiving the message about DS_Store while in every folder there's a .gitignore with DS_Store* in it.
Is there an other way of removing the DS_Store files?

patrickd’s picture

patrickd commented

I think files that are already "git add"'ed wont be ignored afterwards. doing "git rm" on these files should work, I think

betawerk’s picture

betawerk commented
Status: Needs work » Needs review

Thanks! Git rm did the job.
-----------------------
Readme.txt added
New branch 7.x-1.x added
Fixed Coder warnings
Removed .DS_Store

ricovandevin’s picture

ricovandevin commented

I reviewed the code for this installation profile and could not find any major issues. Some small things I found include

  • Text indent is not consistent in
    • tedx_installation_profile.install on lines 33 - 37, line 46 and lines 250 - 279
    • themes/tedx/template.php on lines 195 - 198
    • themes/tedx/theme-settings.php lines 153 - 156
    • all tpl.php files in the themes template folder
  • I guess some development code has to be removed from the themes template.php file on lines 6 - 12

I've not yet been able to test the installation profile. I'll do so as soon as possible.

betawerk’s picture

betawerk commented

Hi rvdvin,

Tnx for reviewing! I changed the text indent on the first three pages and removed the development code.

betawerk’s picture

betawerk commented

Also fixed the text indent for the tpl.php files!

ricovandevin’s picture

ricovandevin commented
Status: Needs review » Reviewed & tested by the community

Tested the installation profile. No problems found.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new drupalcs-result.txt175.94 KB

You have not listed any reviews of other project applications in your issue summary as strongly recommended here: http://drupal.org/node/1011698

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

manual review:

  1. "Update" is not a very helpful commit message, see http://drupal.org//node/52287
  2. Superfish is a whitelisted library on http://drupal.org/packaging-whitelist , so remove it from your repository.
  3. your installation profile (info file etc.) should be in the root folder of your repository, remove the subfolders.
  4. I think you should shorten your profile name from "tedx_installation_profile" to just "tedx_profile" (less redundancy).
  5. template.php: All functions should have proper doc blocks.
  6. template.php: don't execute PHP globally, add the CSS in an appropriate hook.
  7. jquery.mousewheel.js appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms. The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org. Same for the other libraries.
  8. All modules/features that export should start with your installation profile name to avoid name collisions, i.e. "text_formats" should be "tedx_profile_text_formats".
  9. I removed the errors for the libraries that you included in the automated review result, so you should take a look at that.
betawerk’s picture

betawerk commented

Tnx for the review klausie,

I have a few questions/reactions:

  1. Totally true, sorry!
  2. Removed superfish from libraries folder and added it to the .make file.
  3. Which subfolders doe you mean? I thought it was the intention to keep the Drupal installation folder structure, with the .make file in the root? So if you execute tedx_profile.make, drush will generate the Drupal installation folder around this file?
  4. Changed projectname and module prefix to tedx_profile.
  5. Added extra doc blocks, i will make the notes make more sense
  6. I've changed the inline css to a simple img tag and used the style.css to get the same result. Is it wrong to use a php line like print file_create_url(theme_get_setting('sublogo_path')); in the page-front.tpl.php?
  7. Removed the 3rd party code
  8. Done
  9. Does a review from PAReview.sh needs to show zero errors?

Tnx in advance!

betawerk’s picture

betawerk commented
Status: Needs work » Needs review

Fixed pareview errors.

Please review and does somebody now how to fix 

sites/all/modules/pareview_temp/./test_candidate/profiles/tedx_profile/themes/tedx/template.php:
+143: [minor] in most cases, replace the string function with the drupal_ equivalent string functions

and 

FILE: ...view_temp/test_candidate/profiles/tedx_profile/themes/tedx/template.php
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
51 | ERROR | Missing parameter type at position 1
54 | ERROR | Data type of return value is missing
104 | ERROR | Missing parameter type at position 1
107 | ERROR | Data type of return value is missing
--------------------------------------------------------------------------------

Tnx! We would really like to make the profile public.

Auke Rick’s picture

Auke Rick commented
Status: Needs review » Reviewed & tested by the community

I don't think the error from pareview is a major issue. Can't find any other issues.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Fixed
  1. There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
  2. Regarding the code sniffer errors: you should use data types after @param and @return, see http://drupal.org/node/1354#functions
  3. Your repository should not contain a full Drupal, only the installation profile files and make files. See http://drupal.org/developing/distributions So your installation profile (info file etc.) should be in the root folder of your repository, remove the subfolders.
  4. tedx_headerbg(): several variables are undefined here.

But that are just minor issues, so ...

Thanks for your contribution, betawerk! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

betawerk’s picture

betawerk commented

Tnx Klausi!

We will be keeping an eye on the project application list and try to review as much application as possible!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Superfish error

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Component: other » distribution/profile
Issue summary: View changes