Problem/Motivation

1. The module should at least provide an installation test, which ensures that the site is not broken (Status 200 / not WSOD) after enabling the module. Futher tests would of course also be helpful.

2. When running a test, it raises the following schema errors:

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for micon.micon.fa with the following errors: micon.micon.fa:archive.0 missing schema, micon.micon.fa:archive.1 missing schema, micon.micon.fa:archive.2 missing schema, micon.micon.fa:archive.3 missing schema, micon.micon.fa:archive.4 missing schema

3. Furthermore, there are some minor coding standard issues:
Coding standards: https://www.drupal.org/pift-ci-job/2319000

Steps to reproduce

Proposed resolution

  1. Add at least some basic tests
  2. Fix schema errors
  3. Fix coding standards

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork micon-3271002

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

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes

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

grevil’s picture

grevil commented
Status: Active » Needs review

I fixed most of the coding style issues, the schema and implemented a basic installation test!

The archive "sequence" was missing, note that I labeled it "Archive content" you can change the label accordingly :)

Grevil opened merge request !5

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Great work @Grevil! Looks perfect! Nice to see the schema fix!

Hopefully the maintainers will also have a look soon. RTBC +1

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

PS: The composer require failure is strange... that must be unrelated to the tests.

As manual composer.json is a problem for Drupal CI in many cases, best would be to remove it, if it's not needed. And let Drupal create it automatically. But that's just my two cent. ;) Perhaps someone knows better.

Local tests work fine, so still RTBC+1

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

  • JaceRider committed 5e138a6 on 2.x authored by Grevil 
    Issue #3271002 by Grevil: Fix coding standards, add installation test...
jacerider’s picture

jacerider commented

I merged this into the dev branch for further testing. Thanks for the patch!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@JaceRider VERY cool! :) Very much looking forward to your feedback :)

(And a new stable release... ;))

grevil’s picture

grevil commented

Thanks, @JaceRider!

grevil’s picture

grevil commented
Status: Needs review » Fixed
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

(But no new release with fixed schema yet, so this is still broken for our dependent module)

Status: Fixed » Closed (fixed)

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

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@JaceRider would it be possible perhaps to have a new release? So we could finish our tests upstream, which are failing due to the schema errors in micon.

I think the schema fixes are important enough for a new release?

Thanks a lot in advance :)