Aegir Custom Settings

+1 from me -- I see only good things coming from hosting this at d.o. The code doesn't appear to be in need of review, as it's a codebase that already has an alpha release. Marking as RBTC to get higher up's opinions.

@claar, the application process still needs to be done. The review process is outlined in http://drupal.org/node/539608. Will appreciate your review.

It seems to me that when applying for full project approval, one should submit a module or a theme. Am I wrong here?

Just to clarify, omega8cc,

1. We definitely want you to have full project access and value you as a contributor.
2. We definitely want Barracuda hosted as a full project on Drupal.org

but...

3. The idea of the "Full project" application is to review a module or a theme and see that it meets expectations for community code. That lets us deal with matters of security, style, and community acceptance.

so...

I'm going to mark this closed (won't fix) and ask you to submit a regular module or theme for review. Then once you get approved, you can promote your many contributions from sandbox to full project.
Sorry for the hassle.

-Randy

And if I'm off-base here somebody correct me.

Note, this is a duplicate of #1054254: omega8cc [omega8cc], but we are continuing with this one and closing the other.

@rfay, there is good concern to not go forward with this application. But we may as well address this sort of issue now. Here's the two options I see.

1) @omega8cc continues with this specific project application, and someone that has expertise in this area does the review. Maybe someone from the Aegir team.
2) @omega8cc starts a new application with a more traditional module that can be reviewed by a larger audience.

@omega8cc, please let us know how you want to proceed.

@claar, thanks for the basic review. But we do need a full review either way. If the code is perfect, still comment how you have looked at it, some of the positive things, and possibly some ideas when taking it forward. The goal is for it to be clear on this issue that you, as a reviewer, have done a thorough job. Please refer to the following page and sub pages for reviewing: http://drupal.org/node/636570

My goal was to get the ball rolling on this application -- it looks like I succeeded :) Thanks for the input, all.

I've read and thought about the review process in relation to this application, and I agree with zzolo's statement:

But we may as well address this sort of issue now.

Here's a small review of things I am able to review:

1. Your Barracuda sandbox already has 141 commits. These commits have useful and concise commit messages, evidencing an experienced developer able to work with others. Consider adopting Drupal commit log best practices, and opening an issue related to each code change as you move forward, which fosters community participation and helps provide further context for changes.
2. Your Barracuda sandbox contains a LICENSE.txt for GPL version 3. LICENSE.txt files are automatically added for you by the package handler, so this should be removed. and you must be willing to license your project under version 2 and later of the GPL to host your project at drupal.org. We hope this isn't a problem for you.
3. We do not currently have code standards for bash scripts, so doing a code-style review is a bit subjective. However, there are some code consistency issues that should be addressed. For example, in Barracuda.sh.txt:
   • Most of the file is indented with two spaces, but tabs are used on lines 367-456, 831, 1259-1262 (in comment). Many other parts of the file from 853 to 1082 are indented with three spaces, and 391-397 use four spaces. The if statement on line 954 isn't indented at all. Drupal php uses two spaces as well, so I'd suggest standardizing on that.
   • Several sections of code seem like they'd benefit from a comment saying what that section does. For example, it isn't quickly clear to me what the section starting at line 2588 is for. Most sections are self-documenting, however

More later I hope! Thanks!

Hey @omega8cc. So, it sounds like you want to continue with this.

Yes, it is kind of an issue that there are only a limited types of projects. This is a much bigger topic than this issue.

I still think we need to get someone that has a little more focused expertise in this area to do the review. This is not usually something we look for in this Full Project Application review, but I think this is appropriate for this specific issue.

@claar, what is your background or experience in this area?

zzolo: While I have sufficient bash and programming knowledge to review such a project, I do not have sufficient time to devote to it due to its large size (11000+ lines of bash, 300+ lines of php, 250+ lines of make files, and these counts themselves are inaccurate due to the complexity of the project).

In fact, I suggest that this project is large enough that we should look to omega8cc's commit history on this project rather than looking at the whole project itself to determine whether to make her a vetted user.

This project is of the size that it needs a community of developers to improve it. It cannot and should not be (fully) polished in this project review process. But I do think it provides enough of an opportunity to evaluate omega8cc as a developer.

It would perhaps be reasonable and helpful to suggest polishing a file or two, such as BARRACUDA.sh.txt, to provide a review opportunity. I can help some with this, and I agree that it'd be nice to invite other Aegir developers to chime in.

But all of this is subjective and off-the-cuff -- our processes certainly don't embrace this sort of thing currently. But it's a reasonable extension, I think.

Omega8cc: Thanks for hanging in there while we figure all of this out. We very much value your contributions and want to make this as painless as possible while respecting (and adapting!) d.o. review processes.

My two cents: someone can submit any kind of project they want to get it reviewed. But this project is so ambitious and so hard to review (it requires configuring so many different things) that it may take forever to get someone to review it.

It could be quicker to submit one part of this (especially if it is a module or some drush code that the reviewers are familiar with) for review and once that part is approved you could create the full project. At that point you can add in all the other pieces assuming that you apply the same standards to those as was applied to the piece that got reviewed.

It's fine to continue here. Thanks for your patience.

Awesome -- thanks, Grace.

I have a performed a code style review of your Aegir Custom Settings project and was unable to find any code-style issues. You follow Drupal indentation and commenting guidelines very well.

The only question/issue I have is that the .module file is empty except for

include_once('aegir_custom_settings.features.inc')

which implements several hooks, and the other .inc files in the project are not referenced anywhere. Why are these implementations not in the .module file, and what's with the extra .inc files? (I'm weak on features -- forgive me if this is standard and I'm missing something).

OK, let's get this show on the road.

OK, Git vetted user role granted. Welcome, and thanks for your contributions (and your patience in this process).

Please review other applications so we can get this queue down.