The Fepper theme for Drupal includes templates generated by the Fepper frontend prototyper as well as styles, scripts, and theme functions which build out the Drupal theme to match the look and feel of the Fepper frontend.

This theme is ideal for incorporating Pattern Lab into your Drupal development flow. There is probably no other theme which provides any reasonably working interface between the two very different environments.

Project URL: https://www.drupal.org/project/fepper

To clone: git clone --branch 7.x-1.x https://git.drupal.org/project/fepper.git

Manual reviews of other projects:
https://www.drupal.org/project/projectapplications/issues/2857851#comment-12033403
https://www.drupal.org/project/projectapplications/issues/2867474#comment-12066142
https://www.drupal.org/project/projectapplications/issues/2867474#comment-12407693
https://www.drupal.org/project/projectapplications/issues/2829866#comment-12409021

Fepper theme

CommentFileSizeAuthor
#5 fepper.jpg71.95 KBSatyam Upadhyay
screenshot.png274.96 KBe2tha-e
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

e2tha-e created an issue. See original summary.

PA robot’s picture

Multiple Applications
It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2882672

Project 2: https://www.drupal.org/node/2868493

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

e2tha-e’s picture

For visibility:
I opened this request for security advisory review specifically for the 7.x-1.x branch of the Fepper theme. I thought since the 7.x-1.x and 8.x-1.x branches have distinct codebases, each would require a separate review. I did not know that opening this would close the request for review of the 8.x-1.x branch.

The 8-x.1-x branch still requires review. The codebase may be reviewed by cloning as follows:
git clone --branch 8.x-1.x https://git.drupal.org/project/fepper.git

Satyam Upadhyay’s picture

Status: Needs review » Needs work
FileSize
71.95 KB

@e2tha-e,

Kindly fix the https://pareview.sh/node/1586 as follows in screenshot.

Regards
Satyam

e2tha-e’s picture

@satyam-upadhyay Thank you kindly for your feedback!

The Code Sniffer has only outputted warnings for README.md, a Markdown file. These warnings are for lines longer than 80 chars, which happen to contain long links. I cannot reasonably format them to be shorter. Furthermore, they are warnings, not errors.

I don't think these warnings require "fixes." Please set this back to "Needs review." Thanks again!

e2tha-e’s picture

Status: Needs work » Needs review
e2tha-e’s picture

Due to lack of response, I'm switching this to "Needs review."

PA robot’s picture

Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgprojectfeppergit

I'm a robot and this is an automated message from Project Applications Scraper.

e2tha-e’s picture

The "unused use statement" error has been fixed for pareview.sh. Moving back to "Needs review."

e2tha-e’s picture

Status: Needs work » Needs review
e2tha-e’s picture

Issue summary: View changes
e2tha-e’s picture

I've added manual reviews as necessary to join the Review Bonus track:

Manual reviews of other projects:
https://www.drupal.org/project/projectapplications/issues/2857851#comment-12033403
https://www.drupal.org/project/projectapplications/issues/2867474#comment-12066142
https://www.drupal.org/project/projectapplications/issues/2867474#comment-12407693
https://www.drupal.org/project/projectapplications/issues/2829866#comment-12409021

Please prioritize this issue to pass the Security Review. This theme is a minimalist theme with very little executable code. It presents no security risks in and of itself.

e2tha-e’s picture

Issue tags: +PAreview: review bonus
apaderno’s picture

Priority: Normal » Critical

To the reviewers: Please set the priority to Normal after reviewing this application.

apaderno’s picture

zymphonies-dev’s picture

Status: Needs review » Needs work

Please find the below Drupal 8 version review comments.

1) There are no regions added in info file, I think it is required to define regions in info file.
#Regions
regions:
#Header
secondary_menu: 'Secondary Menu'

2) In-completed schema config - You have just written type and label for the schema.

3) It is good organize template files into multiple sub-folders.
Example:
-- templates
---- block
------ block related twig files.

apaderno’s picture

Priority: Critical » Normal
e2tha-e’s picture

@zymphonies-team

Thanks for the feedback! I'll get on this and update as necessary.

e2tha-e’s picture

@zymphonies-team

Re: Drupal 8 version review

1. Per drupal.org, the regions block of the info.yml file is optional. Another drupal.org page states:

If you declare any regions in your theme, even just one, all the default regions will no longer be applied and you assume responsibility for declaring any and all regions you want to use.

Since I'm sticking with the defaults, it's better to leave the regions undeclared.

2. I've deleted config/schema/fepper.schema.yml since it doesn't do anything with just "type" and "label" declared.

3. The latest revision of the D8 Fepper theme only has 12 templates. This number is unlikely to rise much, as this theme is designed to be a base theme. With this goal in mind, I would argue that adding folders adds to complexity instead of reducing it.

e2tha-e’s picture

Status: Needs work » Needs review
zymphonies-dev’s picture

Great, Proceed with other reviews.

sleitner’s picture

Title: [D7] Fepper » [D8] Fepper
Status: Needs review » Needs work

Automated Review

Pareview details: https://pareview.sh/pareview/https-git.drupal.org-project-fepper.git/rev...

Git errors:

Review of the 8.x-1.x branch (commit 9459d57):

This automated report was generated with PAReview.sh, your friendly project application review script.

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
No: Does not follow the licensing requirements.
3rd party assets/code
No: Does not follow the guidelines for 3rd party assets/code. Install icons in /libraries folder
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
Yes: Meets the security requirements.
Coding style & Drupal API usage
see pareview details

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

sleitner’s picture

Issue summary: View changes
apaderno’s picture

Title: [D8] Fepper » [D7] Fepper
Issue summary: View changes
Status: Needs work » Needs review

e2tha-e applied using the Drupal 7 version of the project; that is the version to review.

sleitner’s picture

Status: Needs review » Needs work

The prefered branch 7.x-1.x is 10 month old and is 7 commits behind of 7.x-2.x.

e2tha-e posted in #4 the branch 8.x-1.x to clone. All previous reviews this year had been for 8.x-1.x.

The issues are the same in 8.x-1.x and in 7.x-1.x: README.txt, licensing and 3rd party assets/code.

apaderno’s picture

See #4:

I opened this request for security advisory review specifically for the 7.x-1.x branch of the Fepper theme. I thought since the 7.x-1.x and 8.x-1.x branches have distinct codebases, each would require a separate review. I did not know that opening this would close the request for review of the 8.x-1.x branch.

Users who open these applications cannot request us to review a branch for a different Drupal version, nor can we arbitrarily move the review from a branch for a Drupal version to a branch for a different Drupal version.
If the user who is applying prefers we review the branch for the newer Drupal version, the user needs to create a new application and close the old one as duplicate.

e2tha-e’s picture

@sleitner
Thanks for you review. I'll make the necessary updates as soon a I am able.

@kiamlaluno and @@sleitner
I originally applied for a D8 security review, and thought I could apply for a D7 security review concurrently. The original D8 review request was closed as duplicate.

In the meantime, I'm updating both branches concurrently, although yes, D7 is behind D8. But I am actively committing locally to D7.

I have little preference for which takes priority, but since this issue is labelled for D7, (and D7 is still the more popular Drupal major version), I would agree to focus on D7. Whatever suggestions were made for D8 will be applied to D7.

e2tha-e’s picture

@sleitner

Addressed Feedback (both D7 and D8)

One word Git commit message
Point taken. That short a commit message is rare in this repo and will be avoided in the future.
Licensing
How does this not follow licensing requirements? The GPL license is correctly added to the released packages. All third-party assets are correctly attributed. What's wrong or missing?
3rd party assets/code
The icon fonts have been removed entirely.
Coding style & Drupal API usage
All warnings regard the readme and will be commented on next.
README.txt/README.md
The recommended headings are in place and all uppercase. They pass PAReview. As for lines with links exceeding 80 characters, those are warnings, and the line lengths cannot be avoided.

I'm putting this back under "Needs review."

e2tha-e’s picture

Status: Needs work » Needs review
sleitner’s picture

Status: Needs review » Needs work

3rd party assets/code: remove the fonts at 7.x-2.x as well

I'm not a license expert, is CC0 compatible with GPL 2.0? And Copyright (C) 2017 - ∞ Electric Eloquence compatible with GPL 2.0 ?

## COPYRIGHT

Copyright (C) 2017 - ∞ Electric Eloquence

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

### The Fepper theme for Drupal bundles the following third-party resources:

#### Composite screenshot image

Copyright: Alex Grichenko

* License: CC0 1.0 Universal (CC0 1.0)
* License URI: https://creativecommons.org/publicdomain/zero/1.0/
* Source: http://www.publicdomainpictures.net/view-image.php?image=50301&picture=sunrise-in-mountains

Copyright: Shaun Pierre

* License: CC0 1.0 Universal (CC0 1.0)
* License URI: https://creativecommons.org/publicdomain/zero/1.0/
* Source: http://www.publicdomainpictures.net/view-image.php?image=40694&picture=blue-mountains

Copyright: Public Domain Archive

* License: CC0 1.0 Universal (CC0 1.0)
* License URI: https://creativecommons.org/publicdomain/zero/1.0/
* Source: http://publicdomainarchive.com/winter-mountain-glacier-grey-blue/

Copyright: Alex Grichenko

* License: CC0 1.0 Universal (CC0 1.0)
* License URI: https://creativecommons.org/publicdomain/zero/1.0/
* Source: http://www.publicdomainpictures.net/view-image.php?image=55734&picture=night-sky-in-mountains
e2tha-e’s picture

e2tha-e’s picture

Status: Needs work » Needs review
sleitner’s picture

Status: Needs review » Reviewed & tested by the community
e2tha-e’s picture

@sleitner Thanks a million!

apaderno’s picture

Assigned: Unassigned » apaderno
Status: Reviewed & tested by the community » Fixed

Thank you for your contribution!

I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

Status: Fixed » Closed (fixed)

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