Support from Acquia helps fund testing for Drupal Acquia logo

Comments

FiNeX created an issue. See original summary.

liquidgnome’s picture

seems like this patch ought to help, at least it helped in my situation. it's my first patch, though, so i couldn't tell you if i did it remotely according to best practice.

friera’s picture

It doesn't work for me. I provide a new patch to solve that.

FiNeX’s picture

The patch works :-)

OnkelTem’s picture

I haven't tried it yet but I don't think that just moving it under admin is the correct approach.

OnkelTem’s picture

Category: Bug report » Support request
Status: Active » Needs review
FileSize
5.14 KB
34.9 KB

Uploading a patch. It works pretty much like node, i.e. on Appearance page added a checkbox to use Admin theme for ECK entities.

Appearance page

Status: Needs review » Needs work

The last submitted patch, 6: eck-admin-theme-2859737-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

OnkelTem’s picture

The previous patch was not finished (missed some edits).

nnevill’s picture

Matroskeen’s picture

Issue tags: +LutskGSW18

Added missing tag :)

  • Matroskeen committed 83ee1f7 on 8.x-1.x authored by nnevill
    Issue #2859737 by OnkelTem, nnevill, liquidgnome, friera: Use admin...
Matroskeen’s picture

Status: Needs review » Needs work

Hello,
It works well for me. Sure, it will be great to have additional tests for this feature.

Let's move to "Needs work" for tests.

NickDickinsonWilde’s picture

Status: Needs work » Needs review
FileSize
1.41 KB

URGENT! As committed, that breaks dev.
There was a new file not committed.
Patch attached, with just that change.

  • Matroskeen committed ed9cec9 on 8.x-1.x authored by NickWilde
    Issue #2859737 by OnkelTem, nnevill, liquidgnome, NickWilde, friera,...

Status: Needs review » Needs work

The last submitted patch, 13: eck-use_admin_form-2859737-13.patch, failed testing. View results

Matroskeen’s picture

My bad, sorry.
@NickWilde, thanks for your reactivity. Committed to 8.x-1.x.

frob’s picture

Status: Needs work » Fixed

From what I can tell this should be closed. @Matroskeen please change if that isn't the case.

Matroskeen’s picture

Actually, I'm not sure what is the best way.

I would like to cover all functionality with tests.
Otherwise, I prefer do not slow down the whole process and commit working stuff without tests.

So, how to deal with this case?
Maybe there is a reason to add child issues for tests only?

Status: Fixed » Closed (fixed)

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