CVS edit link for shushu

I am working on a domain+rules module, to be able to CRUD-like domains via Rules.
The existing domain_actions does not fit properly to rules, and does not enable CRUD actions over the domain themselves.
CommentFileSizeAuthor
#9 domain_rules.tgz2.01 KBshushu
#4 domain_rules.tgz2.12 KBshushu
#1 domain_rules.tgz1.24 KBshushu

Comments

shushu’s picture

Status: Postponed (maintainer needs more info) » Needs review
Issue tags: +token, +domain access, +rules
StatusFileSize
new1.24 KB

First version/draft for domain_rules.
Currently only creation of a domain exists (and works properly).
Use case - create a domain when a user creates a node.
While domain_user only create user-specific domains, which is limited, this module is flexible using rules+tokens.

avpaderno’s picture

Status: Needs review » Needs work
Issue tags: -token, -domain access, -rules +Module review

Hello, and thanks for applying for a CVS account. I am adding the review tags, and some volunteers will review your code, pointing out what needs to be changed.

As per http://drupal.org/cvs-application/requirements, the motivation message should be expanded to contain more details about the features of the proposed module, and it should include also a comparison with the existing solutions.

mitchell’s picture

I can vouch for his code. Looks great!

You can see my gratitude in #423800-4: Provide example foreign UI.

Best of luck, shushu.

shushu’s picture

Status: Needs work » Needs review
StatusFileSize
new2.12 KB

Hello,
Hope to get the approval this time...
Attached a new version of my new module - domain_rules.tgz.

The motivation for this module is to add the following Rules actions:

  • rules_action_create_domain - create new domain using given arguments
  • rules_action_delete_domain - delete existing domain
  • rules_action_set_user_defaults_domain - set a given role access to a given domain (as described in #782838: Document: "Add default roles dynamically" to dynamically add new subdomains).
  • rules_action_set_node_domain - Set domain to a node. The difference in this action from the existing one is the integration with Token
  • rules_action_set_domain_theme - set a theme to a given domain

Combining all of those actions together makes a much stronger capabilities than existing automatic domain creation such as domain_user module.

Hope this clears the motivation, and hope my code is good enough for the CVS.
Comments are more than welcome.

shushu’s picture

Any news ?
How can I get a working cvs account ?

Almost a month from my first submittion.

Regards,
Shushu

Exploratus’s picture

Would love to know the status...

mitchell’s picture

Status: Needs review » Reviewed & tested by the community

I have reviewed the submitted code. This application can be approved.

Updating status according to cvs application workflow.

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » Needs work
  1.         include_once drupal_get_path('module', 'domain') .'/domain.admin.inc';
    

    There is a Drupal function to use, in such cases.

  2. Strings used in the user interface should be in sentence case.
  3. See http://drupal.org/coding-standards to understand how a module should be written. In particular, see how the code should be formatted, and how constants should be written.
  4. Some comments make me think the code is not complete. Could you provide a more complete code?
  5. Did you propose a patch to include this code into domain.module?
shushu’s picture

Status: Needs work » Needs review
StatusFileSize
new2.01 KB

Thanks for the detailed feedback.
5. I suggested it for domain.module, and was answered "The 6.x branch is closed to new features. " #782838: Document: "Add default roles dynamically" to dynamically add new subdomains. Still, I think many will appreciate those features, and since I plan to find time to maintain it, I see no reason why not to publish it.
4. TODO notes were for myself. All been removed.
3-1. Done, thanks.
I also used the Coder module, which was very useful to pinpoint the issues.

Please approve my account so I will be able to publish it.

Regards,
Shushu

avpaderno’s picture

Status: Needs review » Fixed

Check the indentation character, as some files don't use the two spaces as suggested in the coding standards.

Thank you for your contribution! I am going to update your account.
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)
Issue tags: -Module review

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

avpaderno’s picture

Component: Miscellaneous » new project application
Issue summary: View changes