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.

Sign up for GitHub

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

Jump to bottom

webrtc-adapter dependency to npm dependency from git submodule #283

Merged
merged 2 commits into from
Aug 30, 2017
Merged

webrtc-adapter dependency to npm dependency from git submodule #283

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da66df7
webrtc-adapter dependency is now an npm dependency rather than a git …
Aug 11, 2017
b0ea0f5
Change webrtc-adapter dependency from compatible to exact version
NTARelix Aug 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .gitmodules
View file
Open in desktop

This file was deleted.

24 changes: 1 addition & 23 deletions Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module.exports = function(grunt) {
grunt.loadNpmTasks('grunt-replace');
grunt.loadNpmTasks('grunt-include-replace');
grunt.loadNpmTasks('grunt-karma');
grunt.loadNpmTasks('grunt-githash');
grunt.loadNpmTasks('grunt-string-replace');

grunt.initConfig({
Expand All @@ -23,22 +22,10 @@ module.exports = function(grunt) {
production: 'publish',
bamboo: 'bamboo',

// webrtc-adapter submodule
webrtcAdapterSubmodule: 'third_party/adapter/',
googleAdapterPath: '<%= webrtcAdapterSubmodule %>/out/adapter.js',

pluginInfoRoot: grunt.option('pluginInfoRoot') || '<%= source %>',
pluginInfoFile: grunt.option('pluginInfoFile') || 'pluginInfo.js',

githash: {
submodule: {
options: {
dir: '<%= webrtcAdapterSubmodule %>'
}
},
},

version: '<%= pkg.version %>-<%= githash.submodule.short %>',
version: '<%= pkg.version %>',
Copy link

Choose a reason for hiding this comment

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

@johache should we keep this as discussed previously @ncurrier?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we would no longer be depending on a git-hash, but rather a node dep' version...
If anything, then what we should put next to our version number would be the webrtc-adapter version number.

I'm not sure if that's worth it though...

Copy link

@oooookk7 oooookk7 Aug 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see it's necessary as well if we are relying on the npm dependency version, but since this might change our release tagging process a bit, its be better we reconfirm with @ncurrier on this.

Copy link
Member

Choose a reason for hiding this comment

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

I'm ok with the version being specified in the package.json as long as it explicitly specified and does not rely on ranges. This PR specifies any minor release greater than 2.0.3 (^2.0.3), that I am not ok with.

@johache are you comfortable with only sticking to published releases? That does limit us slightly from previously where its commit based, however that may not be an issue and the simplicity may be favored.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with this limitation. We can always revert in the future if we realise this is completely blocking for us.
It does sound safer to only use released version anyway. It means that it passed Google's safety checks.

@NTARelix , could you change your PR so that it uses a specific version rather than a compatible one ? (2.0.3 instead of ^2.0.3)


clean: {
production: ['<%= production %>/'],
Expand Down Expand Up @@ -323,13 +310,6 @@ module.exports = function(grunt) {
grunt.log.writeln('bamboo/vars file successfully created');
});

grunt.registerTask('CheckSubmodules', 'Checks that third_party/adapter is properly checked out', function() {
if(!grunt.file.exists(grunt.config.get('googleAdapterPath'))) {
grunt.fail.fatal('Couldn\'t find ' + grunt.config.get('googleAdapterPath') + '\n' +
'Output would be incomplete. Did you remember to initialize submodules?\nPlease run: git submodule update --init');
}
});

grunt.registerTask('CheckPluginInfo', 'Checks for existing config file', function() {
var fullPath = grunt.config.get('pluginInfoRoot') + '/' + grunt.config.get('pluginInfoFile');
grunt.verbose.writeln('Checking that the plugin info file exists.');
Expand All @@ -356,8 +336,6 @@ module.exports = function(grunt) {
});

grunt.registerTask('publish', [
'githash',
'CheckSubmodules',
'CheckPluginInfo',
// 'webrtc-adapter',
'versionise',
Expand Down
12 changes: 4 additions & 8 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,10 @@ AdapterJS.extensionInfo = {
```

## Setup this project
1. Copy this repository with submodules (`git clone --recursive ...`), or run `git submodule init` and `git submodule update`.
2. Install or update to at lest version `0.10.26` of node and version `1.4.6 `of npm.
3. Install `grunt-cli`. (See: http://gruntjs.com/getting-started)
4. Run `npm install` to install dev dependencies.
5. Run `npm install -g browserify` and `npm install -g testling` (might require sudo) to install the necessary tools to test locally
1. Install or update to at lest version `0.10.26` of node and version `1.4.6 `of npm.
2. Install `grunt-cli`. (See: http://gruntjs.com/getting-started)
3. Run `npm install` to install dev dependencies.
4. Run `npm install -g browserify` and `npm install -g testling` (might require sudo) to install the necessary tools to test locally


## Development
Expand Down Expand Up @@ -344,8 +343,5 @@ You can also run `grunt karma` to run the test and bypass the publish step.

(Mac only) If you are testing the Temasys WebRTC Plugin, you can run `osascript tests/mac.watcher.scpt` to automatically validate the permission popup.

##### `third_party/`
The `webrtc/adapter` dependency linked repo at commit version.

## License
APACHE 2.0 - http://www.apache.org/licenses/LICENSE-2.0.html
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
}
],
"scripts": {
"postinstall": "npm --prefix third_party/adapter install third_party/adapter",
"version": "grunt publish",
"prepublish": "grunt publish",
"test": "./test.sh;"
Expand Down Expand Up @@ -55,7 +54,8 @@
"karma-safari-launcher": "^0.1.1",
"karma-script-launcher": "0.1.0",
"mocha": "2.1.0",
"requirejs": "^2.1.19"
"requirejs": "^2.1.19",
"webrtc-adapter": "^2.0.3"
},
"testling": {
"files": "tmp/*.js",
Expand Down
Loading