Problem/Motivation

The module forces installation/enabling of unnecessary packages/modules:

  1. Require of drupal/printable forces require of smalot/pdfparser but it is only needed for tests.
  2. Enabling printable forces enabling of pdf_api but not actually required.

Proposed resolution

  1. Change smalot/pdfparser to be require-dev.
  2. Remove pdf_api from printable.info.yml and add it to composer.json.
CommentFileSizeAuthor
#3 printable.dependency.3213681-3.patch583 bytesadamps
#2 printable.dependency.3213681-2.patch577 bytesadamps

Comments

AdamPS created an issue. See original summary.

adamps’s picture

adamps commented
Status: Active » Needs review
StatusFileSize
new printable.dependency.3213681-2.patch577 bytes
adamps’s picture

adamps commented
StatusFileSize
new printable.dependency.3213681-3.patch583 bytes
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

  • jcnventura authored 2a9cd4b on 2.x 
    Issue #3213681 by AdamPS, jcnventura: Avoid unnecessary dependencies
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Reviewed & tested by the community » Fixed
adamps’s picture

adamps commented

Great thanks, happy to see this project is still active

pahles’s picture

pahles commented

Since you no longer require pdf_api maybe the description of the project should be updated.

jcnventura’s picture

jcnventura
Primary language English
commented

It does require pdf_api to generate PDFs which is a sub-module. The description is still accurate.

pahles’s picture

pahles commented

Looking at the source code there is a submodule printable_pdf, not pdf_api. When I did a composer update in my project pdf_api was removed.

jcnventura’s picture

jcnventura
Primary language English
commented
Title: Avoid unnecessary dependencies » Move pdf_api dependency to composer.json
Status: Fixed » Active

The submodule printable_pdf depends on pdf_api. Which of course now is broken on sites that install the latest dev since pdf_api is no longer installed.

The solution for this (at least for now), is to add the pdf_api module as a composer dependency, so that it is installed by composer.json, while allowing the change here to do it's objective which is to be able to enable the printable module without having to enable the optional PDF functionality.

jcnventura’s picture

jcnventura
Primary language English
commented
Status: Fixed » Active

The patch in #3 is correctly adding the pdf_api dependency, but something must have happened when I created the commit and that line was not applied.

  • AdamPS authored efdec24 on 2.x 
    Issue #3213681 by AdamPS, jcnventura: Move pdf_api dependency to...
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

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