cordova-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CB-12397) .gitignore fix
Date Fri, 01 Jun 2018 15:09:00 GMT

    [ https://issues.apache.org/jira/browse/CB-12397?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16498105#comment-16498105
] 

ASF GitHub Bot commented on CB-12397:
-------------------------------------

brodybits closed pull request #8: CB-12397 fix .gitignore for plugins & platforms (cordova-create
part)
URL: https://github.com/apache/cordova-create/pull/8
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/index.js b/index.js
index 61181ca..05ba2f0 100644
--- a/index.js
+++ b/index.js
@@ -213,6 +213,17 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents)
{
                 fs.mkdirSync(dir);
             }
 
+            // CB-12397 add .gitignore for plugins & platforms
+            // to generated app
+            // (use stock .npmignore for ignore file)
+            // NOTE: This is a workaround solution for the existing
+            // npm .gitignore/.npmignore behavior discussed in:
+            // npm/npm#1862
+            // npm/npm#3763
+            // npm/npm#7252
+            const ignoreStockPath =
+                path.join(require('cordova-app-hello-world').dirname, '.npmignore');
+
             try {
                 // Copy files from template to project
                 if (cfg.lib.www.template) {
@@ -263,6 +274,16 @@ module.exports = function (dir, optionalId, optionalName, cfg, extEvents)
{
 
                 pkgjson.version = DEFAULT_VERSION;
                 fs.writeFileSync(pkgjsonPath, JSON.stringify(pkgjson, null, 4), 'utf8');
+
+                // XXX TODO USE fs-extra function instead,
+                // XXX WAITING FOR apache/cordova-create#14 to be merged
+                // CB-12397 add .gitignore for plugins & platforms
+                // to generated app
+                // XXX GONE:
+                shell.cp(ignoreStockPath, path.join(dir, '.gitignore'));
+                /* ** XXX TODO:
+                fs.copySync(ignoreStockPath, path.join(dir, '.gitignore'));
+                // */
             }
 
             // Create basic project structure.
diff --git a/spec/create.spec.js b/spec/create.spec.js
index 68e924a..50aed3b 100644
--- a/spec/create.spec.js
+++ b/spec/create.spec.js
@@ -57,80 +57,130 @@ describe('cordova create checks for valid-identifier', function () {
 
 describe('create end-to-end', function () {
 
-    function checkProject () {
+    function checkProjectCommonArtifacts () {
         // Check if top level dirs exist.
         var dirs = ['hooks', 'platforms', 'plugins', 'www'];
         dirs.forEach(function (d) {
             expect(path.join(project, d)).toExist();
         });
 
+        // Check that README.md exists inside of hooks
         expect(path.join(project, 'hooks', 'README.md')).toExist();
 
-        // Check if www files exist.
+        // Check that index.html exists inside of www
         expect(path.join(project, 'www', 'index.html')).toExist();
 
-        // Check that config.xml was updated.
-        var configXml = new ConfigParser(path.join(project, 'config.xml'));
-        expect(configXml.packageName()).toEqual(appId);
-
-        // TODO (kamrik): check somehow that we got the right config.xml from the fixture
and not some place else.
-        // expect(configXml.name()).toEqual('TestBase');
-    }
+        // Check if config.xml exists.
+        expect(path.join(project, 'config.xml')).toExist();
 
-    function checkConfigXml () {
-        // Check if top level dirs exist.
-        var dirs = ['hooks', 'platforms', 'plugins', 'www'];
-        dirs.forEach(function (d) {
-            expect(path.join(project, d)).toExist();
-        });
-        expect(path.join(project, 'hooks', 'README.md')).toExist();
-
-        // index.js and template subdir folder should not exist (inner files should be copied
to the project folder)
+        // index.html, index.js and template subdir folder
+        // should not exist in top level
+        // (inner files should be copied to the project top level folder)
+        expect(path.join(project, 'index.html')).not.toExist();
         expect(path.join(project, 'index.js')).not.toExist();
         expect(path.join(project, 'template')).not.toExist();
 
-        // Check if www files exist.
-        expect(path.join(project, 'www', 'index.html')).toExist();
+        // Check that .gitignore does not exist inside of www
+        expect(path.join(project, 'www', '.gitignore')).not.toExist();
+
+        // Check that .npmignore does not exist inside of www
+        expect(path.join(project, 'www', '.npmignore')).not.toExist();
+
+        // Check that config.xml does not exist inside of www
+        expect(path.join(project, 'www', 'config.xml')).not.toExist();
+
+        // Check that no package.json exists inside of www
+        expect(path.join(project, 'www', 'package.json')).not.toExist();
+
+        // Check that config.xml was updated.
         var configXml = new ConfigParser(path.join(project, 'config.xml'));
         expect(configXml.packageName()).toEqual(appId);
         expect(configXml.version()).toEqual('1.0.0');
+        // Check that we got the right config.xml from the fixture and not some place else.
+        expect(configXml.name()).toEqual('TestBase');
+    }
 
-        // Check that config.xml does not exist inside of www
-        expect(path.join(project, 'www', 'config.xml')).not.toExist();
+    function checkProjectArtifactsWithConfigFromTemplate () {
+        checkProjectCommonArtifacts();
+
+        // Check that standard js artifact does not exist
+        expect(path.join(project, 'www', 'js')).not.toExist();
+        expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+        // [CB-12397] Check that .gitignore does not exist
+        expect(path.join(project, '.gitignore')).not.toExist();
+        // [CB-12397] Check that .npmignore does not exist
+        expect(path.join(project, '.npmignore')).not.toExist();
 
         // Check that we got no package.json
         expect(path.join(project, 'package.json')).not.toExist();
 
         // Check that we got the right config.xml from the template and not stock
+        const configXml = new ConfigParser(path.join(project, 'config.xml'));
         expect(configXml.description()).toEqual('this is the correct config.xml');
     }
 
-    function checkSubDir () {
-        // Check if top level dirs exist.
-        var dirs = ['hooks', 'platforms', 'plugins', 'www'];
-        dirs.forEach(function (d) {
-            expect(path.join(project, d)).toExist();
-        });
-        expect(path.join(project, 'hooks', 'README.md')).toExist();
+    function checkProjectArtifactsWithNoPackageFromTemplate () {
+        checkProjectCommonArtifacts();
 
-        // index.js and template subdir folder should not exist (inner files should be copied
to the project folder)
-        expect(path.join(project, 'index.js')).not.toExist();
-        expect(path.join(project, 'template')).not.toExist();
+        // Check that standard js artifact does not exist
+        expect(path.join(project, 'www', 'js')).not.toExist();
+        expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+        // [CB-12397] Check that .gitignore does not exist
+        expect(path.join(project, '.gitignore')).not.toExist();
+        // [CB-12397] Check that .npmignore does not exist
+        expect(path.join(project, '.npmignore')).not.toExist();
+
+        // Check that we got no package.json
+        expect(path.join(project, 'package.json')).not.toExist();
+    }
+
+    function checkProjectArtifactsWithPackageFromTemplate () {
+        checkProjectCommonArtifacts();
+
+        // Check that standard js artifact exists
+        expect(path.join(project, 'www', 'js')).toExist();
+        expect(path.join(project, 'www', 'js', 'index.js')).toExist();
+
+        // Check if package.json exists.
+        expect(path.join(project, 'package.json')).toExist();
+
+        // [CB-12397] Check that .gitignore exists
+        expect(path.join(project, '.gitignore')).toExist();
+        // [CB-12397] Check that .npmignore exists
+        expect(path.join(project, '.npmignore')).toExist();
+
+        // Check that we got package.json (the correct one)
+        var pkjson = requireFresh(path.join(project, 'package.json'));
+        // Pkjson.displayName should equal config's name.
+        expect(pkjson.displayName).toEqual('TestBase');
+    }
+
+    function checkProjectArtifactsWithPackageFromSubDir () {
+        checkProjectCommonArtifacts();
+
+        // Check that standard js artifact does not exist
+        expect(path.join(project, 'www', 'js')).not.toExist();
+        expect(path.join(project, 'www', 'js', 'index.js')).not.toExist();
+
+        // [CB-12397] Check that .gitignore exists
+        expect(path.join(project, '.gitignore')).toExist();
+        // [CB-12397] Check that .npmignore does not exist
+        expect(path.join(project, '.npmignore')).not.toExist();
 
         // Check if config files exist.
         expect(path.join(project, 'www', 'index.html')).toExist();
 
-        // Check that config.xml was updated.
-        var configXml = new ConfigParser(path.join(project, 'config.xml'));
-        expect(configXml.packageName()).toEqual(appId);
-        expect(configXml.version()).toEqual('1.0.0');
         // Check that we got package.json (the correct one)
         var pkjson = requireFresh(path.join(project, 'package.json'));
+
         // Pkjson.displayName should equal config's name.
         expect(pkjson.displayName).toEqual(appName);
         expect(pkjson.valid).toEqual('true');
 
         // Check that we got the right config.xml
+        const configXml = new ConfigParser(path.join(project, 'config.xml'));
         expect(configXml.description()).toEqual('this is the correct config.xml');
     }
 
@@ -138,7 +188,7 @@ describe('create end-to-end', function () {
         // Create a real project with no template
         // use default cordova-app-hello-world app
         return create(project, appId, appName, {}, events)
-            .then(checkProject)
+            .then(checkProjectArtifactsWithPackageFromTemplate)
             .then(function () {
                 var pkgJson = requireFresh(path.join(project, 'package.json'));
                 // confirm default hello world app copies over package.json and it matched
appId
@@ -161,7 +211,7 @@ describe('create end-to-end', function () {
                 expect(fetchSpy).toHaveBeenCalledTimes(1);
                 expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url);
             })
-            .then(checkProject);
+            .then(checkProjectArtifactsWithPackageFromTemplate);
     });
 
     it('should successfully run with NPM package', function () {
@@ -179,7 +229,7 @@ describe('create end-to-end', function () {
                 expect(fetchSpy).toHaveBeenCalledTimes(1);
                 expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url);
             })
-            .then(checkProject);
+            .then(checkProjectArtifactsWithPackageFromTemplate);
     });
 
     it('should successfully run with NPM package and explicitly fetch latest if no version
is given', function () {
@@ -198,7 +248,7 @@ describe('create end-to-end', function () {
                 expect(fetchSpy).toHaveBeenCalledTimes(1);
                 expect(fetchSpy.calls.argsFor(0)[0]).toBe(config.lib.www.url + '@latest');
             })
-            .then(checkProject);
+            .then(checkProjectArtifactsWithPackageFromTemplate);
     });
 
     it('should successfully run with template not having a package.json at toplevel', function
() {
@@ -211,7 +261,7 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkProject)
+            .then(checkProjectArtifactsWithNoPackageFromTemplate)
             .then(function () {
                 // Check that we got the right config.xml
                 var configXml = new ConfigParser(path.join(project, 'config.xml'));
@@ -229,7 +279,7 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkProject);
+            .then(checkProjectArtifactsWithNoPackageFromTemplate);
     });
 
     it('should successfully run with template having package.json, and subdirectory, and
no package.json in subdirectory', function () {
@@ -242,7 +292,7 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkProject);
+            .then(checkProjectArtifactsWithNoPackageFromTemplate);
     });
 
     it('should successfully run with template having package.json, and subdirectory, and
package.json in subdirectory', function () {
@@ -255,7 +305,7 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkSubDir);
+            .then(checkProjectArtifactsWithPackageFromSubDir);
     });
 
     it('should successfully run config.xml in the www folder and move it outside', function
() {
@@ -268,7 +318,7 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkConfigXml);
+            .then(checkProjectArtifactsWithConfigFromTemplate);
     });
 
     it('should successfully run with www folder as the template', function () {
@@ -281,13 +331,20 @@ describe('create end-to-end', function () {
             }
         };
         return create(project, appId, appName, config, events)
-            .then(checkConfigXml);
+            .then(checkProjectArtifactsWithConfigFromTemplate)
+            .then(() => {
+                // Additional check that we have the fixture www,
+                // not one from stock the app
+                expect(path.join(project, 'www', 'fixture-marker-page.html')).toExist();
+                expect(path.join(project, 'www', 'subdir')).toExist();
+                expect(path.join(project, 'www', 'subdir', 'sub-fixture-marker-page.html')).toExist();
+            });
     });
 
     it('should successfully run with existing, empty destination', function () {
         shell.mkdir('-p', project);
         return create(project, appId, appName, {}, events)
-            .then(checkProject);
+            .then(checkProjectArtifactsWithPackageFromTemplate);
     });
 
     describe('when --link-to is provided', function () {
@@ -315,6 +372,7 @@ describe('create end-to-end', function () {
 
                 // Check www/config exists
                 expect(path.join(project, 'www', 'config.xml')).toExist();
+
                 // Check www/config.xml was not updated.
                 var configXml = new ConfigParser(path.join(project, 'www', 'config.xml'));
                 expect(configXml.packageName()).toEqual('io.cordova.hellocordova');
@@ -323,6 +381,7 @@ describe('create end-to-end', function () {
 
                 // Check that config.xml was copied to project/config.xml
                 expect(path.join(project, 'config.xml')).toExist();
+
                 configXml = new ConfigParser(path.join(project, 'config.xml'));
                 expect(configXml.description()).toEqual('this is the correct config.xml');
                 // Check project/config.xml was updated.
diff --git a/spec/helpers.js b/spec/helpers.js
index daf6722..b15486c 100644
--- a/spec/helpers.js
+++ b/spec/helpers.js
@@ -85,9 +85,9 @@ beforeEach(function () {
                     result.pass = fs.existsSync(testPath);
 
                     if (result.pass) {
-                        result.message = 'Expected file ' + testPath + ' to exist.';
-                    } else {
                         result.message = 'Expected file ' + testPath + ' to not exist.';
+                    } else {
+                        result.message = 'Expected file ' + testPath + ' to exist.';
                     }
 
                     return result;
diff --git a/spec/templates/config_in_www/www/fixture-marker-page.html b/spec/templates/config_in_www/www/fixture-marker-page.html
new file mode 100644
index 0000000..6a9821c
--- /dev/null
+++ b/spec/templates/config_in_www/www/fixture-marker-page.html
@@ -0,0 +1 @@
+<h1>Fixture marker page</h1>
diff --git a/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html
new file mode 100644
index 0000000..311ec82
--- /dev/null
+++ b/spec/templates/config_in_www/www/subdir/sub-fixture-marker-page.html
@@ -0,0 +1 @@
+<h1>Sub-fixture marker page</h1>


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> .gitignore fix
> --------------
>
>                 Key: CB-12397
>                 URL: https://issues.apache.org/jira/browse/CB-12397
>             Project: Apache Cordova
>          Issue Type: Improvement
>          Components: cordova-app-hello-world, cordova-cli, cordova-create, cordova-lib
>            Reporter: Chris Brody
>            Priority: Major
>              Labels: backlog, easy-fix
>
> Followup to CB-12008 (autosave by default in cordova@7): if a user creates an app using
"cordova create" there should be a .gitignore file to exclude the plugins and platforms artifacts
from git.
> I raise this since I have seen way too many apps with outdated plugins / platforms artifacts
included in git.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org


Mime
View raw message