It is still using old drupal 8 PSR-0 standard for directory. It should be changed to PSR-4 (as core is using).

CommentFileSizeAuthor
#14 2548659-14.patch4.02 KBjoshi.rohit100
#12 2548659-11.patch3.75 KBjoshi.rohit100
#10 2548659-10.patch4.11 KBjoshi.rohit100
#2 2548659-2.patch1.32 KBjoshi.rohit100

Comments

joshi.rohit100 created an issue. See original summary.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Active » Needs review
StatusFileSize
new 2548659-2.patch1.32 KB

Here is the the patch.

Status: Needs review » Needs work

The last submitted patch, 2: 2548659-2.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented

I think, we should handler the failed tests in separate issues as currently module is not in right condition.

Thoughts ?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Yes please, if you have some interest in it please open a new issue for fixing the D8 tests.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented

Yes I am interested. but I am confused as whether we should first fix the testcases or should fixe the structure as currently it is not working. If I open new issue, then that will depend on this one ?

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Related issues: +#2548731: Fix failing testcases
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Related issues: -#2548731: Fix failing testcases

Opened new issue for testcases https://www.drupal.org/node/2548731

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

@joshi.rohit100: If you find that you can't fix the tests without first fixing the path, feel free to just work from this one and can the other one - it may be a catch-22 situation. Also, thanks!

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2548659-10.patch4.11 KB

I think, I should fix the test failures (only existing) here.

Also I couldn't find 'twitter_mock' module, so I just removed that module from test for now.

Status: Needs review » Needs work

The last submitted patch, 10: 2548659-10.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2548659-11.patch3.75 KB

Test are failing because wrong implementation of twitter entity (EntityNG class is currently not there in code).

So I think its fine.

Thoughts ?

Status: Needs review » Needs work

The last submitted patch, 12: 2548659-11.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new 2548659-14.patch4.02 KB

Status: Needs review » Needs work

The last submitted patch, 14: 2548659-14.patch, failed testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #2583115: Identify replacements for Twitter on Drupal 8

Committed. Thanks.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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