We could be using README.md instead of README.txt to make use of the nicer rendering via Markdown.

Now that we've moved to GitLab, we should probably use Markdown instead of plain .txt files for our documentation.

CommentFileSizeAuthor
#24 WITH MR.png323.35 KBkenyoowen
#24 without MR.png311.14 KBkenyoowen
#9 login.png168.66 KBnupur badola
#4 3319899-4.patch4.29 KBnitapawar
#2 3319899-2.patch4.34 KBnitapawar

Issue fork login_redirect_per_role-3319899

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

nitapawar created an issue. See original summary.

nitapawar’s picture

nitapawar commented
Status: Active » Needs review
StatusFileSize
new 3319899-2.patch4.34 KB
Manoj Raj.R’s picture

Manoj Raj.R commented
Status: Needs review » Needs work

After reviewing this patch this needs some work to be done.
Please remove and follow the markdown format.
+-----------

nitapawar’s picture

nitapawar commented
Status: Needs work » Needs review
StatusFileSize
new 3319899-4.patch4.29 KB

Updated patch!

Manoj Raj.R’s picture

Manoj Raj.R commented
Status: Needs review » Reviewed & tested by the community

Reviewed this patch.

#4 patch looks good to me.
Found nil error.

can be moved to RTBC.

Manoj Raj.R opened merge request !9

Manoj Raj.R’s picture

Manoj Raj.R commented
Status: Reviewed & tested by the community » Needs review

Updated the Readme File as per the latest format.
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

Please review.

Manoj Raj.R’s picture

Manoj Raj.R commented
Assigned: nitapawar » Unassigned
nupur badola’s picture

nupur badola commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new login.png168.66 KB

Patch is working and marking it to RTBC.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Needs review

It would be better if somebody else reviews the MR.

nupur badola’s picture

nupur badola commented

Reviewed 3319899-4.patch, the patch is not cleanly applied.

1. Introduction format is incorrect and not as per readme.md file template.
2. Requirement section is missing.
3. Two lines prior to ##/### headings.
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

The format to use in a README.md file and which information must be provided is described in README.md template.

nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: Unassigned » nitin_lama
nitin_lama’s picture

nitin_lama
he/him
Location India
commented
Assigned: nitin_lama » Unassigned
Status: Needs work » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

Anjali Mehta made their first commit to this issue’s fork.

anjali mehta’s picture

anjali mehta commented
Status: Needs work » Needs review

Kindly Review the changes
Thank You.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work
anjali mehta’s picture

anjali mehta commented
Status: Needs work » Needs review

Kindly review the changes.
Thank you

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

sakthi_dev made their first commit to this issue’s fork.

sakthi_dev’s picture

sakthi_dev commented
Status: Needs work » Needs review

Please review.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: Replace README.txt with READEME.md format » Replace README.txt with README.md
Issue summary: View changes
kenyoowen’s picture

kenyoowen commented
Status: Needs review » Needs work
StatusFileSize
new without MR.png311.14 KB
new WITH MR.png323.35 KB

Hi sakthi_dev

I applied and reviewed your MR!9, but the configuration section is still not fixed. As per Apaderno "It is not necessary to say In menu, which is also missing a definite article." maybe you can change it to something like "Navigate to /admin/people/login-and-logout-redirect-per-role and set up login or logout tables for redirect URL or what to set in the configuration page.". Please see the screenshots attached.

Thank you.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

I also added further comments in another review I have done.

anjali mehta’s picture

anjali mehta commented
Status: Needs work » Needs review

jeroent made their first commit to this issue’s fork.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs review » Fixed

Merged to 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

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