Profolio is a clean and neutral theme designed and created for my D7 portfolio-blog (dutch). Sidebars are optional. - Demo
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | drupalcs-result.txt | 1.7 KB | klausi |
| profolioscreen.png | 54.54 KB | dave.l |
Comments
Comment #2
jordojuice commentedUpdating priority according to priority guidelines. The application's priority should be set back to normal once a reviewer responds to your application and the application review process has continued.
Comment #3
dave.l commentedOk Thx for the info. Setting this project back to normal.
Comment #4
jordojuice commentedSorry, once a reviewer responds and gives you information on what work (if any) needs to be done. We want your application to have an elevated priority until then since it has been waiting without sufficient response for a long period of time. Your application can have a higher priority because of this, so enjoy it! : )
Comment #5
mjohnq3 commentedVery nice theme. I did notice that some of the CSS doesn't fully comply with Drupal's CSS Coding Standards. For example:
This:
h2 {font-size: 1.3em;}
should be:
h2 {
font-size: 1.3em;
}And this, at least per the standards and despite being logically related styles, is a bit hard to read:
#footer a{color: #CCC;}
#footcontainer h2{background-color: #333;}
#topcontent .block h2, #sidebar_first .block h2, #sidebar_last .block h2{
color: #415369;
margin: 0px;
font-weight: bold;
padding-top:0px;
border-bottom-width: 1px;
border-bottom-style: solid;
border-bottom-color: #cccccc;
}I am aware that many Drupal themers don't follow these standards to the letter, so this is more informational than critical.
Comment #6
dave.l commented"Alphabetizing properties" seems a bit harsh.. but if its the standard, then i will set it up correctly! will get back in a couple of days
Thanks for the info mjohnq3
Comment #7
dave.l commentedUploaded the altered version a couple of days ago. CSS should be according to the Drupal standards. deleted some unnessecary files (images)
re-check plz!
Comment #8
mjohnq3 commentedReviewed CSS files. Many changes made for compliance with Drupal Coding standards. All files pass Coder module review. Hopefully one of the official project reviewers will take a look at this.
Comment #10
dgoutam commentedChanged priority as per http://drupal.org/node/894256
* README.txt is missing
* Remove all old CVS $Id tags, not needed anymore
* what is index.php and index.html for?
* looks like you included a third party library, is this needed? see the policy: http://drupal.org/node/422996
* template.php: function names should start lower cased.
* functions should be document with doc blocks, what hook they implement etc.
Comment #12
dave.l commentedSorry for the late respons
3th party library is deleted
added readme...
..In short, applied all of the above!
Thx for your time Klausi
Review of the 7.x-1.x branch:
- Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards):
FILE: ...e/drupal-7/sites/all/modules/pareview_temp/test_candidate/block.tpl.php -------------------------------------------------------------------------------- FOUND 3 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 2 | ERROR | Missing file doc comment 2 | ERROR | Concat operator must be surrounded by spaces 2 | ERROR | Concat operator must be surrounded by spaces -------------------------------------------------------------------------------- FILE: .../sites/all/modules/pareview_temp/test_candidate/comment-wrapper.tpl.php -------------------------------------------------------------------------------- FOUND 2 ERROR(S) AFFECTING 2 LINE(S) -------------------------------------------------------------------------------- 3 | ERROR | Missing file doc comment 9 | ERROR | String concat is not required here; use a single string instead -------------------------------------------------------------------------------- FILE: ...drupal-7/sites/all/modules/pareview_temp/test_candidate/comment.tpl.php -------------------------------------------------------------------------------- FOUND 6 ERROR(S) AFFECTING 2 LINE(S) -------------------------------------------------------------------------------- 1 | ERROR | Missing file doc comment 1 | ERROR | Concat operator must be surrounded by spaces 1 | ERROR | Concat operator must be surrounded by spaces 39 | ERROR | Concat operator must be surrounded by spaces 39 | ERROR | Concat operator must be surrounded by spaces 39 | ERROR | Concat operator must be surrounded by spaces -------------------------------------------------------------------------------- FILE: ...ce/drupal-7/sites/all/modules/pareview_temp/test_candidate/html.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 2 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...sites/all/modules/pareview_temp/test_candidate/maintenance-page.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 3 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...ce/drupal-7/sites/all/modules/pareview_temp/test_candidate/node.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 2 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...drupal-7/sites/all/modules/pareview_temp/test_candidate/overlay.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 1 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...ce/drupal-7/sites/all/modules/pareview_temp/test_candidate/page.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 5 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...all/modules/pareview_temp/test_candidate/region--footer_message.tpl.php -------------------------------------------------------------------------------- FOUND 3 ERROR(S) AFFECTING 2 LINE(S) -------------------------------------------------------------------------------- 1 | ERROR | You must use "/**" style comments for a file comment 1 | ERROR | Whitespace found at end of line 2 | ERROR | Inline comments must end in full-stops, exclamation marks, or | | question marks -------------------------------------------------------------------------------- FILE: .../drupal-7/sites/all/modules/pareview_temp/test_candidate/region.tpl.php -------------------------------------------------------------------------------- FOUND 1 ERROR(S) AFFECTING 1 LINE(S) -------------------------------------------------------------------------------- 1 | ERROR | Missing file doc comment -------------------------------------------------------------------------------- FILE: ...ce/drupal-7/sites/all/modules/pareview_temp/test_candidate/template.php -------------------------------------------------------------------------------- FOUND 6 ERROR(S) AFFECTING 6 LINE(S) -------------------------------------------------------------------------------- 1 | ERROR | You must use "/**" style comments for a file comment 5 | ERROR | You must use "/**" style comments for a function comment 10 | ERROR | else must start on a new line 18 | ERROR | You must use "/**" style comments for a function comment 26 | ERROR | else must start on a new line 36 | ERROR | You must use "/**" style comments for a function comment -------------------------------------------------------------------------------- - README.txt is missing, see the guidelines for in-project documentation.
- Remove all old CVS $Id tags, they are not needed anymore.
css/print.css:1:/* $Id$ */ css/ie_styles.css:1:/* $Id$ */ - All text files should end in a single newline (\n). See http://drupal.org/node/318#indenting
./css/local.css ./css/ie_styles.css ./css/style.css ./node.tpl.php ./comment.tpl.php ./maintenance-page.tpl.php ./page.tpl.php ./comment-wrapper.tpl.php
This automated report was generated with PAReview.sh, your friendly project application review script. Go and review some other project applications, so we can get back to yours sooner.
Comment #14
patrickd commentedSwitched back to needs review, so in-depth reviews won't be blocked by coding standart issues.
Comment #15
dave.l commented-added comments to the files like descriped in the standards
-tryed to fix the concat problems, in some files i found the problem, in others it was a '.' att the end of a string (i guess)
-added a new line to the end of each file
-checked the indenting (maybe i missed some, hope not)
-removed $id from the css files
10 | ERROR | else must start on a new line
on the template.php file i can not find a else statement inline (all are on a new line).
...of to investigate the Sniffer module
THX again klausi!
Comment #16
dave.l commentedA couple of hours later (and about 10 commits) the files are clean according to Sniffer (7.x-1.x).
(used a online version att: http://ventral.org/ )
| Status | File | Size |
|---|---|---|
| new | drupalcs-result.txt | 1.7 KB |
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:
- Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See attachment.
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. Go and review some other project applications, so we can get back to yours sooner.
maual review:
- "Initial release" is not a good commit message, especially if repeated again and again. See http://drupal.org/node/52287
- profolio_preprocess_page(): this is a hook implementation and should be documented as such. See http://drupal.org/node/1354#hookimpl
Otherwise looks RTBC to me.
Comment #19
dave.l commentedsubmitted some commits. used better Issue description on commits, codesniffers report is empty, Master branch is empty.
Thx Klausi
Comment #20
dave.l commentedForgot to change the Issue status to 'needs review'.
Thanks for your contribution, Dave.L! Welcome to the community of project contributors on drupal.org.
kiamlaluno has 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.
Automatically closed -- issue fixed for 2 weeks with no activity.
I am giving credits to the users who participated in this issue.
Automatically closed - issue fixed for 2 weeks with no activity.
Comment #1
dave.l commentedhttp://drupal.org/sandbox/Dave.L/1169892
Forgot to include the sandbox link ;)