Posted by nomonstersinme on September 1, 2009 at 5:03pm
| Project: | Drupal.org CVS applications |
| Component: | Miscellaneous |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | theme review |
Issue Summary
CVS edit link for nomonstersinme
My sister Jacine opened me up to the Drupal community just over a year ago and I have been working in it since. I come from a design background and after attending Design4Drupal and constantly hearing about needing more and better designs for Drupal. I am working really hard on a couple of themes that I would like to contribute. This is why I am applying for a CVS account.I have one theme up at http://drupal.amandarodriguez.com, it is not 100% complete but I thought I would submit the URL so you can see that I am working on something.
Comments
#1
#2
#3
<?phpif (is_file(drupal_get_path('theme', 'agua') .'/preprocess/preprocess-'. str_replace('_', '-', $hook) .'.inc')) {
include('preprocess/preprocess-'. str_replace('_', '-', $hook) .'.inc');
}
?>
The path is relative to the Drupal root directory; the INCLUDE-statement (as it is not a function, the parentheses are optional) will not find the file.
drupal_strtolower()versusstrtolower()).#4
#5
@nomonstersinme: When you upload new code, you change the status to ; only who approves your CVS application can change the status to fixed.
#6
version = 1.x-devThat line should be removed, as it is added by the packaging script.
#7
can you clarify #2... what files are from third party sites?
#8
The file I am referring is superfish.js. If it is not available from third-party sites is a file from a different project; in both the cases, it should be committed in Drupal.org CVS.
#9
i don't understand. It is available from a 3rd party site, under gpl license and also exists in other drupal.org projects... but why does that mean I'm not allowed to use it?
#10
Any files that are available from third-party sites should not committed to Drupal.org CVS, which should used only for Drupal modules.
If your module require a third-party library (or file), the users of your module needs to download it from the other site.
See 3rd party libraries in Drupal CVS.
#11
ok then......
#12
scripts[] = js/superfish.jsThe line makes reference to a not existing file.
#13
oops my bad! thanks!
#14
quoting KiamLaLuno
This is not always the case.
If this theme depends on a specific version of superfish, then it's appropriate to include it with the contrib.
For example, Drupal core includes jquery.js
#15
Has this person's CVS account been approved?
While the code review is possibly useful, it should not be tied -- and certainly should not be a pre-requisite -- for a CVS account being approved or not.
Setting to "needs review" --
@nomonstersinme -- can you let us know if your application has been approved or not? If your app has been approved, then we can set this to fixed.
#16
I really cant see whats the problem is with including the suckerfish.js if its under GPL or should a themer wait for someone to release a suckerfish module - before it can be used in a theme?
#17
subscribe
(and what kind of welcome for a designer is this?)
#18
Fusion (http://drupal.org/project/fusion), put out by Top Notch Themes, the same company that gives us "Acquia Marina", "Acquia Prosper" and "Acquia Slate", uses Superfish, and I am pretty sure I didn't have to download it.
#19
I checked out nomonstersinme's account and looks like the CVS application has been approved.
superfish.js, being part of a jQuery plugin SuperFish, is not easily downloaded by itself (you have to download the entire package to get the one file). Seeing as it's already GPL2-licensed and using common sense, there would be no problem including it in the theme's CVS repository.
As far as code reviews, when you're one of only a few people doing CVS application review, trying to read every person's differently-formatted code can turn a 5 minute task into much more. That said, if things are so bad that we can't read the module/theme's code, I wouldn't want to approve the application. But if things are generally readable and make an attempt to follow the standards (aka it should pass coder.module looking for major violations), it shouldn't be a blocker.
I can see that this has more potential to fuel the designers vs. developers argument. So why don't we get a few more design-oriented volunteers to help review CVS applications for themes? I repeat that this CVS issue queue is understaffed enough. If we need to improve our CVS application text or what to expect with the review process, please let us know.
nomonstersinme, I'd like to welcome you to being a contributing CVS member of the community. This is a big step for a lot of people, so congrats! The screenshots of your theme look very nice and hope that lots of our end users find your theme useful.
#20
Yeah, I'm a bit surprised by this as well. KiamLaLuno, while it's nice that you are reviewing and helping with the theme, wouldn't that be best done in the theme's issue queue when the account is approved and the theme is submitted? It seems nomonstersinme has followed all of the guidelines necessary for approval of the CVS account, so it should be approved.
If I were a designer and submitting a new theme and received these somewhat responses without much explanation and little to no cordiality or respect, I would likely move elsewhere. Just something to think about.
Nice work on contributing your theme nomonstersinme! Looking forward to seeing it up on Drupal.org and more to come in the future...
#21
Seriously?! This is a fantastic theme. Stop the hassling. But the account has already been approved.
#22
Oops, sorry for the status change. Looks like David and I submitted around the same time.
#23
The Fusion theme does indeed include Superfish.js. Unfortunately, the rule against including third-party libraries is well-founded, and it's hard to justify applying it selectively. This should should be raised and discussed somewhere -- just not here.
That said, I want to apologize to nomonstersinme on behalf of the folks on Drupal.org. Having your work kicked back to you in a technical manner -- using legal language, at that -- only to be told that you were still doing something wrong by not including the file that moments ago you were told to remove is not how we should welcome people into our community.
As we bring more creative people into the fold, we need to understand that not everyone understands the blunt, terse world of development. Single-line answers with little explanation or context are a huge turnoff. That kind of behavior will ultimately turn people away.
Let's make an effort to explain why a certain policy is in place. Making someone ask and wait for a response is deflating. It makes them feel confused and ignorant.
#24
Sorry -- didn't realize this was a CVS application. Changing title back.
#25
Lol...so many cross-posts. :)
#26
When I set the status to fixed it means I approve the CVS application.
#27
Welcome nomonstersinme, looking forward to seeing some more themes from you :)
If you need any help with CVS, give me a shout, although you're probably covered if Jacine has been sharing her knowledge with you.
#28
wow, I should have checked this thread earlier.. thanks for the support guys! its really appreciated!
I was a big caught off guard at first, but I understand rules are rules.
Hoping to finish xbrowser css and get the theme up this weekend!
#29
Automatically closed -- issue fixed for 2 weeks with no activity.