Support from Acquia helps fund testing for Drupal Acquia logo

Comments

renatog created an issue. See original summary.

renatog’s picture

Assigned: renatog » Unassigned
Status: Active » Needs review
FileSize
3.05 KB

The patch it's in attachment.

Good Work and Good Week.

Regards.

krina.addweb’s picture

Status: Needs review » Needs work

@renatog, thanks for the patch it works well & following are the issues which were founded from Readme patch.

1)

INSTALLATION
------------

Install as you would normally install a contributed Drupal module. See:
https://drupal.org/documentation/install/modules-themes/modules-7 for further
information.

1) Copy the flag directory to the modules folder in your installation.

2) Enable the module using Administer -> Modules (/admin/modules)

Change Administer -> Modules (/admin/modules) to Administration -> Modules

2)

CONFIGURATION
-------------

The configuration for Flag is spread between Views configuration and the Flag
site building page. To configure:

1) Configure the flags for your site at
   Administer -> Structure -> Flags (/admin/structure/flags)

   You can create and edit flags on this page. Descriptions for the various
   options are provided below each field on the flag edit form.

2) Go to the Views building pages at
   Administer -> Site Building -> Views (/admin/structure/views)

Change Administer -> Structure -> Flags (/admin/modules) to Administration -> Structure -> Flags

3) Current maintainers: Add "https://www.drupal.org/u/fago" fago as maintainer

dhruveshdtripathi’s picture

Assigned: Unassigned » dhruveshdtripathi

Thank you @krina.addweb for suggesting changes. Working on it.

dhruveshdtripathi’s picture

Made changes according to comment #3. Removed whitespace which was giving a warning while applying the last patch.

dhruveshdtripathi’s picture

Assigned: dhruveshdtripathi » Unassigned
Status: Needs work » Needs review
renatog’s picture

Status: Needs review » Reviewed & tested by the community

Thank you @krinaaddweb for your review and @dhruveshdtripathi for your contribution.

Works Good for me. +RTBC

Regards.

joachim’s picture

Version: 7.x-3.x-dev » 8.x-4.x-dev
Status: Reviewed & tested by the community » Needs work

Thanks everyone for working on this! (And thanks for making me aware of the README standard -- I wasn't aware of it.)

However, work should be done on the 8.x-4.x branch first and then backported.

Also, a few small nitpicks:

  1. +++ b/README.txt
    -- Token, which is required for Flag to provide tokens on flagged entities.
     1) Copy the flag directory to the modules folder in your installation.
     
    -2) Enable the module using Administer -> Modules (/admin/modules)
    +2) Enable the module using Administer -> Modules
    

    These lines can be removed, per the README Template docs.

  2. +++ b/README.txt
    -- Token, which is required for Flag to provide tokens on flagged entities.
    + * Optional Installation
    +   - The ability for anonymous users to flag content is provided by the Session
    +     API module, available at http://drupal.org/project/session_api.
    

    This should go under requirements, surely?

  3. +++ b/README.txt
    @@ -99,3 +128,10 @@ software or obligated in any way to correct problems you may experience.
     Licensed under the GPL 2.0.
     http://www.gnu.org/licenses/gpl-2.0.txt
     
    

    The docs don't mention the need for this, so let's remove it. d.org adds the licence file anyway.

dhruveshdtripathi’s picture

Assigned: Unassigned » dhruveshdtripathi

Working on it

dhruveshdtripathi’s picture

Assigned: dhruveshdtripathi » Unassigned
Status: Needs work » Needs review
FileSize
2.52 KB

Created a patch for 8.x-4.x branch. Made few changes according to standards.

joachim’s picture

Status: Needs review » Needs work

Looking good, thanks!

The section on installation still needs to be changed to the standard form suggested in the template.

dhruveshdtripathi’s picture

Assigned: Unassigned » dhruveshdtripathi

Yes, installation section still needs to be changed to the standard form. Working on it.

Thanks!

dhruveshdtripathi’s picture

Assigned: dhruveshdtripathi » Unassigned
Status: Needs work » Needs review
FileSize
2.87 KB
755 bytes

Made changes in INSTALLATION section according to template.

Following points are taken care in the patch:

- Bullets denoted by asterisks (*) with hanging indents.
- Numbered lists indented 4 spaces.

Thank you!

joachim’s picture

Status: Needs review » Needs work

> Made changes in INSTALLATION section according to template.

That change seems to have been missed off.

The template has this:

* Install as you would normally install a contributed Drupal module. See:
https://drupal.org/documentation/install/modules-themes/modules-7
for further information.

The latest patch still has more detail than is needed.

neel24’s picture

Status: Needs work » Needs review
FileSize
4.33 KB

Ok so I've created a new patch which applies the readme, following the readme template: https://www.drupal.org/docs/develop/documenting-your-project/readme-temp...

neerajpandey’s picture

#15 looks fine. It just needs some spacing edits and support request section. Adding a patch and interdiff for the same.

alonaoneill’s picture

Status: Needs review » Needs work
  1. +++ b/README.md
    @@ -17,82 +16,68 @@ node, comment, user, or any entity type. You may define as many of these 'flags'
     
    

    Add "To submit bug reports and feature suggestions, or to track changes" part here

  2. +++ b/README.md
    @@ -17,82 +16,68 @@ node, comment, user, or any entity type. You may define as many of these 'flags'
    + * Go to Admin > Structure > Flags, and click "Add flag".
    ...
    + * Select the target entity type, and click "Continue".
    ...
    + * Enter the flag link text, link type, and any other options.
    ...
    + * Click "Save Flag".
    ...
    + * Under Admin > People, configure the permissions for each Flag.
    

    Make it numbered list?

  3. +++ b/README.md
    @@ -17,82 +16,68 @@ node, comment, user, or any entity type. You may define as many of these 'flags'
    +
    ...
    +
    ...
    +
    ...
    +
    

    I would remove those empty lines

neel24’s picture

Alright, so I've created a new patch based on the one in #16 and applied the changes mentioned in #17. I've also attached an interdiff.

alonaoneill’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed the patch for spelling and grammar. Formatting aligns to Drupal documentation standards. Everything looks good. Patch applied.
Marking as RTBCed.
Thank you for working on the module!

Berdir’s picture

Status: Reviewed & tested by the community » Fixed

Added myself as a maintainer and committed.

  • Berdir committed d1671b8 on 8.x-4.x authored by neel24
    Issue #2862099 by dhruveshdtripathi, neel24, neerajpandey, RenatoG:...

Status: Fixed » Closed (fixed)

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