NPM Resource Changed ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
12 Oct 2019

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Run npm i etc. Still get all the files in the media and templates

Expected result

Actual result

Documentation Changes Required

NO

But why?

When I was writing the build tools was extremely cautious to the number of dependencies that were required as I thought the less the faster the CI. In the meantime the project added quite a few more dependencies so it makes zero sense to keep the build tools lean if the same mentality is not shared across the board. Most importantly and quite honestly it should solve a problem on my other PR

@wilsonge

avatar dgrammatiko dgrammatiko - open - 12 Oct 2019
avatar dgrammatiko dgrammatiko - change - 12 Oct 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 12 Oct 2019
Category JavaScript Repository NPM Change
avatar C-Lodder
C-Lodder - comment - 12 Oct 2019

Why use mkdirp over fs (native)?
Just an FYI, you're creating directories with 0777 chmod permissions. Doesn't Joomla use 0755?

avatar dgrammatiko
dgrammatiko - comment - 12 Oct 2019

@C-Lodder mkdirp Is creating all the given folders recursively. About permissions I was thinking how that thing will possibly react in the windows system but since I don’t do dos anymore I decided to leave the mode as is. You can test this with mode: 0644 And let me know

avatar C-Lodder
C-Lodder - comment - 12 Oct 2019

Check my template repo. Comitted something yesterday to build the folders recursively using FS

avatar dgrammatiko dgrammatiko - change - 12 Oct 2019
Labels Added: NPM Resource Changed ?
e855edc 12 Oct 2019 avatar dgrammatiko 🐶
avatar dgrammatiko dgrammatiko - change - 13 Oct 2019
The description was changed
avatar dgrammatiko dgrammatiko - edited - 13 Oct 2019
avatar dgrammatiko
dgrammatiko - comment - 13 Oct 2019

@C-Lodder nice but it seems that fs-extra already has this functionality so neither own code or another package is needed for recursive folder creation

avatar C-Lodder
C-Lodder - comment - 13 Oct 2019

Shame Node don't use fs-extra natively. fs has some very annoying flaws in it :/

avatar wilsonge
wilsonge - comment - 13 Oct 2019

@brianteeman can you give this a quick run when you have a minute to ensure this still works on windows please

avatar brianteeman
brianteeman - comment - 13 Oct 2019

Will do after the ?

avatar wilsonge
wilsonge - comment - 13 Oct 2019

@SniperSister @rdeutz can you guys have a look at the ATT why drone isn't triggering for the last 2 builds of this PR please

avatar dgrammatiko dgrammatiko - change - 13 Oct 2019
Title
[4.0] Build tools fixes
[4.0] [NO CACHE] Build tools fixes
avatar dgrammatiko dgrammatiko - edited - 13 Oct 2019
avatar brianteeman
brianteeman - comment - 13 Oct 2019

@wilsonge npm i ran fine on windows. even seemed to be faster than before. Only thing to note is the following. No idea if its related to this PR or not so posting it

npm WARN acorn-dynamic-import@4.0.0 requires a peer of acorn@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN babel-loader@6.4.1 requires a peer of webpack@1 || 2 || ^2.1.0-beta || ^2.2.0-rc but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@5.0.1 requires a peer of webpack@^2.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN com_media@4.0.0 No repository field.
avatar dgrammatiko
dgrammatiko - comment - 13 Oct 2019

@brianteeman yes somehow we need to remove the node_modules in administrator/com_media. I can do it here or spin another PR

avatar HLeithner HLeithner - change - 14 Oct 2019
The description was changed
avatar HLeithner HLeithner - edited - 14 Oct 2019
avatar rdeutz
rdeutz - comment - 14 Oct 2019

Just quick because ....

The volume cache plugin reacts to special directives in a commit message to provide additional functionality. Include one of the following directives somewhere in your commit message to trigger their respective actions:

[CLEAR CACHE]
instruct the plugin to clear the entire cache. This only influences the restoring step, the plugin will still rebuild cache if instructed to do so.
[NO CACHE]
instruct the plugin not to restore or rebuild cache for this build.

avatar dgrammatiko dgrammatiko - change - 14 Oct 2019
Title
[4.0] [NO CACHE] Build tools fixes
[4.0] Build tools fixes
avatar dgrammatiko dgrammatiko - edited - 14 Oct 2019
avatar wilsonge wilsonge - change - 16 Oct 2019
Title
[4.0] Build tools fixes
[4.0][NO CACHE] Build tools fixes
avatar wilsonge wilsonge - edited - 16 Oct 2019
avatar wilsonge
wilsonge - comment - 16 Oct 2019

@dgrammatiko looks like a legit drone failure now

avatar wilsonge wilsonge - change - 16 Oct 2019
Title
[4.0][NO CACHE] Build tools fixes
[4.0][CLEAR CACHE] Build tools fixes
avatar wilsonge wilsonge - edited - 16 Oct 2019
avatar dgrammatiko
dgrammatiko - comment - 16 Oct 2019

But ESLint exists in the package.json

avatar wilsonge
wilsonge - comment - 16 Oct 2019

It's mkdirp that's missing from the dependencies according to the error?

f67c4f2 16 Oct 2019 avatar dgrammatiko oops
avatar wilsonge wilsonge - change - 17 Oct 2019
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-10-17 10:23:37
Closed_By wilsonge
avatar wilsonge wilsonge - close - 17 Oct 2019
avatar wilsonge wilsonge - merge - 17 Oct 2019
avatar wilsonge
wilsonge - comment - 17 Oct 2019

Thanks!

Add a Comment

Login with GitHub to post a comment