Problem/Motivation

The Flag module contains a variety of files whose purpose is to support development. Things like linting rules, automated tests, and package management for dev dependencies have no real purpose for an end user downloading a release.

Proposed resolution

According to the documentation for creating releases, you can exclude these types of files from the final release artifact by including a .gitattributes file.

Issue fork flag-3489058

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

kevin.dutra created an issue. See original summary.

kevin.dutra opened merge request !103

kevin.dutra’s picture

kevin.dutra commented
Assigned: kevin.dutra » Unassigned
Status: Active » Needs review
ivnish’s picture

ivnish
he/him
commented
Status: Needs review » Postponed

del

ivnish’s picture

ivnish
he/him
commented
Status: Postponed » Active

ivnish’s picture

ivnish
he/him
commented
Status: Active » Fixed
berdir’s picture

berdir
Primary language German
Location Switzerland
commented

never seen a project remove test files, are we certain that's a good idea? Core doesn't do that either. Might also affect Gitlab CI?

ivnish’s picture

ivnish
he/him
commented

Gitlab CI is not affected, I checked it.

I haven't seen this before, but the documentation does describe it https://www.drupal.org/node/1068944#s-excluding-non-production-code-from...

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

I'm specifically talking about the tests folder.

What about a project that extends the flag module and has tests using a trait or base class from Flag? If it's not explicitly depending on the de version, it will get a tagged release through composer without the tests folder.

ivnish’s picture

ivnish
he/him
commented

Yeah, looks like there might be more problems than benefits. Let me revert this commit.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Yeah. Also note that Drupal already has a global flag that will not discover or find modules in tests folders which is off by default, in that case the runtime and disk overhead of this is neglectable.

  • ivnish committed 9d49d868 on 8.x-4.x 
    Revert "Issue #3489058: Remove non-production code from release"

This...
ivnish’s picture

ivnish
he/him
commented
Status: Fixed » Closed (works as designed)
ivnish’s picture

ivnish
he/him
commented

Thanks @Berdir!

kevin.dutra’s picture

kevin.dutra commented
Status: Closed (works as designed) » Needs review

I can understand the use case for leaving the test files in, but can we still remove the other cruft? The main reason I had opened this is that our automated security scans of production infrastructure kept flagging issues among the non-prod files, like packages defined in the Yarn lock file that had open flaws.

ivnish’s picture

ivnish
he/him
commented

@kevin.dutra I removed some JS garbage here https://git.drupalcode.org/project/flag/-/commit/1aa7bc96950e16b127d126a...

kevin.dutra’s picture

kevin.dutra commented
Status: Needs review » Closed (outdated)

Ah! Okay, I missed that. Thanks @ivnish!