Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completely refactor build system #205

Merged
merged 1 commit into from
Jul 18, 2019

Conversation

raphinesse
Copy link
Contributor

@raphinesse raphinesse commented Jun 20, 2019

Motivation and Context

The goals of this refactoring are the following:

  • Improve code quality of the build system
  • Prepare code base for adding code coverage
  • Prepare code base for exposing the build system as a package export, so platforms can
    build their cordova.js during their build (or creation) process.
    This would make the JS build using coho obsolete.
  • Prepare code base to remove dependency on Grunt

Description

The build process now lives under build-tools (was tasks). It does
not depend on Grunt anymore but is written in plain Node.js.

The original Grunt interface for building (as used by coho) was
preserved and is implemented in Gruntfile.js using the new build
system. The platformName option and support for getting platform paths
from the cordova-platforms key in package.json have been removed
from the Grunt interface, but neither of those are used in coho.

The logic that is specific to the test build has been extracted from
the rest of the build. It now lives in test/build.js and is run
automatically during npm test.

The following dependencies have been added:

  • execa
  • fs-extra
  • globby

Testing

I have taken extra care to preserve the exact format of the built file.
This means that the correctness of the refactoring can be verified by
simply diffing the build artifacts in pkg created with and without
this change.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change

The goals of this refactoring are the following:

- Improve code quality of the build system
- Prepare code base for adding code coverage
- Prepare code base for exposing the build system as a package export,
  so platforms can build their `cordova.js` during their build (or
  creation) process. This would make the JS build using coho obsolete.
- Prepare code base to remove dependency on Grunt

The build process now lives under `build-tools` (was `tasks`). It does
not depend on Grunt anymore but is written in plain Node.js.

The original Grunt interface for building (as used by coho) was
preserved and is implemented in `Gruntfile.js` using the new build
system. The `platformName` option and support for getting platform paths
from the `cordova-platforms` key in `package.json` have been removed
from the Grunt interface, but neither of those are used in coho.

The logic that is specific to the test build has been extracted from
the rest of the build. It now lives in `test/build.js` and is run
automatically during `npm test`.

The following dependencies have been added:

- execa
- fs-extra
- globby

I have taken extra care to preserve the exact format of the built file.
This means that the correctness of the refactoring can be verified by
simply diffing the build artifacts in `pkg` created with and without
this change.
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@raphinesse raphinesse merged commit 5e417b2 into apache:master Jul 18, 2019
@raphinesse raphinesse deleted the build-refactor branch July 18, 2019 11:15
raphinesse added a commit to raphinesse/cordova-js that referenced this pull request Jul 22, 2019
In apache#205 I took great care to preserve the _exact_ format of the built
JS code, so the build refactoring could be verified by diffing the
build output. However, some of the existing formatting is sub-optimal
so this commit optimizes it a little.

Most notably, this commit will change file path comments to be relative
paths, not absolute ones. An example for this:

```diff
-// file: /home/raphinesse/code/cordova-android/cordova-js-src/exec.js
+// file: ../cordova-android/cordova-js-src/exec.js
```
raphinesse added a commit that referenced this pull request Jul 22, 2019
In #205 I took great care to preserve the _exact_ format of the built
JS code, so the build refactoring could be verified by diffing the
build output. However, some of the existing formatting is sub-optimal
so this commit optimizes it a little.

Most notably, this commit will change file path comments to be relative
paths, not absolute ones. An example for this:

```diff
-// file: /home/raphinesse/code/cordova-android/cordova-js-src/exec.js
+// file: ../cordova-android/cordova-js-src/exec.js
```
@raphinesse raphinesse mentioned this pull request Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants