CommentFileSizeAuthor
#8 3062602-port_drupal-8.patch34.46 KBrahulbaisanemca
#6 drupal-8-porting_3062602_6.patch22.76 KBrahulbaisanemca
#4 0001-drupal-8-porting.patch53.43 KBrahulbaisanemca
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

nerdstein created an issue. See original summary.

rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
Assigned: Unassigned » rahulbaisanemca
rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
FileSize
0001-drupal-8-porting.patch53.43 KB
nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
CreditAttribution: nerdstein as a volunteer and at Hook 42 commented
Status: Active » Needs work

Thank you for the patch, this is looking really good! The patch file may have some issues, it appeared to show the `git status` output as part of the patch, which may fail if we try to merge.

The following are some comments:

1. I did not see a config/install schema for `tugboat.settings` which was mentioned in the config form.

2. Please feel free to remove the license file, as this will get added by drupal.org

3. Please file a ticket to create the tugboat_example submodule at a later date. This is not urgent right now.

4. Please add a comment for the TugboatService class

5. Please rename the "_tugboat_execute" function in TugboatService to just "execute" and update uses of this, e.g. cron in .module

6. I didn't see any use of the permissions found in tugboat.permissions.yml, should we remove it? What is it intended to do?

rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
FileSize
drupal-8-porting_3062602_6.patch22.76 KB

All changes implemented.
Please have a look on this patch.

Thank you

nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
CreditAttribution: nerdstein as a volunteer and at Hook 42 commented

I may be wrong, but that patch seems to be missing a lot of the new files, like .info.yml and several others.

Can you please make a new patch against the diff from 7.x-1.x?

rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
FileSize
3062602-port_drupal-8.patch34.46 KB
rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented

Thank you

nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
CreditAttribution: nerdstein as a volunteer and at Hook 42 commented
Status: Needs work » Reviewed & tested by the community

This patch looks great. Thank you so much. Merging

nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
CreditAttribution: nerdstein as a volunteer and at Hook 42 commented
Status: Reviewed & tested by the community » Fixed

Merged! Thanks

matt westgate’s picture

matt westgate CreditAttribution: matt westgate at Lullabot commented

Keep up the great work, Rahul! Thank you for your contributions.

rahulbaisanemca’s picture

rahulbaisanemca CreditAttribution: rahulbaisanemca as a volunteer and at Proventeq Limited commented
Issue summary: View changes

Thank you Matt

Status: Fixed » Closed (fixed)

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