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

Create first version of release task [#508] #528

Merged
merged 44 commits into from
Jun 3, 2015
Merged

Create first version of release task [#508] #528

merged 44 commits into from
Jun 3, 2015

Conversation

ellisonleao
Copy link
Contributor

No description provided.

stringFiles += filenames[i] + ' ';
}
}
run('git add -f ' + stringFiles, 'Adding build files');
Copy link
Collaborator

Choose a reason for hiding this comment

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

the force option worries me slightly. Could this include files we have ignored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. this only includes files from the filenames array.

@ellisonleao
Copy link
Contributor Author

@obiot @parasyte are we ok with this one?

@obiot
Copy link
Member

obiot commented Jul 10, 2014

seems ok to me, but as i'm not an expert on this topic, I would rather transfer my "approval" to @parasyte , ahahah :):):)

@obiot
Copy link
Member

obiot commented Jul 16, 2014

@parasyte we need your advice on this one too, as you are kind of the expert here :)

@ellisonleao
Copy link
Contributor Author

Hey guys, any news from that? @obiot @parasyte

@parasyte
Copy link
Collaborator

@ellisonleao Ok, let me check it out real quick. :)

'use strict';
var path = require('path');
var Q = require('q');
var shell = require('shelljs');
Copy link
Collaborator

Choose a reason for hiding this comment

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

q and shelljs need to be added to package.json devDependencies.

@parasyte
Copy link
Collaborator

@ellisonleao w00t! Done! The task is really really simple and concise. I like it a lot! I left a few comments for you to address.

grunt.log.oklns('BUILD FILES');
var filenames = [
path.join(buildPath, 'melonjs-' + version + '.js'),
path.join(buildPath, 'melonjs-' + version + '-min.js')
Copy link
Collaborator

Choose a reason for hiding this comment

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

For these filenames, you should be able to use config.path.main and config.path.min.

@obiot
Copy link
Member

obiot commented Jul 21, 2014

wow, I would have never noticed all that :P

@ellisonleao
Copy link
Contributor Author

Nice @parasyte ! I will make the changes and commit it. Thanks for the feedback.

var splitted = symbolicRef.split('/');
// symbolic ref must contain 3 steps
// e.g ref/heads/master
if (splitted.length !== 3) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is possible (and recommended, in some cases!) to name branches with a forward slash. This check would be better like >= 3

Then the currBranch assignment below would be like:

currBranch = splitted.slice(2).join("/");

@parasyte
Copy link
Collaborator

@ellisonleao I have a few more items for you, here. :) Thanks for the ping.

@obiot
Copy link
Member

obiot commented Aug 16, 2014

awesome, would be to sad to miss the 1.1.0 train on this one, as it's 99% complete :):):):)

* upstream/master: (28 commits)
  [#553] small update for code consistency
  [#554] fixed circle/ellipse drawing in debug mode
  [#553] added a very simple pooling mechanism to the quadtree implementation
  [#552] fixed shape drawing in debug mode when additional shapes are being used
  Slight update to the #550 fix (Sorry Aaron!)
  Throw an exception if getContext is not supported
  Fix docs for Error classes
  Fix whitespace
  Update exceptions to use the new `me.Error` class
  closes #550 - adds own canvas to calculate ratio
  Clarified documentation for the `setCollisionMask` function
  small update too for the whack-a-mole example
  Small fix and tweaks to the platformer example
  [#441] Fix bad merge
  [#544] debugPanel : properly draw regions when debug mode is enabled for the QuadTree
  [#544] added a `insertContainer` function to the quadtTree (faster insertion) and added back recursion case with child containers
  directly refer to `api when accessing these properties, shorter and faster
  [#548] Fix default Body bounds when a collision shape is not explicitly defined.
  Create me.game.defaultCollisionMap with proper bounds
  Fix jshint:afterConcat task
  ...
@ellisonleao
Copy link
Contributor Author

Hey guys, thanks for the feedback, i just changed the task with @parasyte 's modifications but i am still having the problem of the task not finishing in the end. I've tried to concatenate the last git command to prevent possible race condition errors, but it remains. @parasyte , any thoughts? thanks for the feedback!

Can't wait to end this one and start studying the new melonjs version.

* upstream/master:
  fixed docs on animation sheet with tps
  [#560] fixed entity position when manually updating their position
  [#560] fixed body default initialization
  [#560] body is not relative to the entity object
  [#560] fixed shape drawing position (in debug mode)
  [#560] fixed the `updateBounds` function when the polyshape default position is offset
  updated stroke* functions to accept a linewidth param.
@obiot
Copy link
Member

obiot commented Sep 9, 2014

any update here ?

* upstream/master: (22 commits)
  [#568] fix the onCollision handler, and use the last response API
  [#568] removed old properties from 1.1.0 (new API must be used now)
  [#568] ResponseObject hidden class changes - part 2
  [#568] fixed ResponseObject hidden class changes
  [documentation] fixed the getShape function return type, as it can never return a me.Rect object
  [538] optimized code, added variable shortcut to avoid "retraversing" the response object again and again
  [#538] calculate residual velocity instead of nullifying it
  [#538] removed the previous tile collision implementation
  [#538] converted the second level, and tweaked the first one a bit
  [#538] clear the falling/jumping flag when required
  Simplified the slope shape
  [documentation] fixed the object scope
  [#538] First commit with world collision shape support
  changed drawImage to apply the arguments, supports all 3 calls of draw image
  and the broken URL !
  fixed missing CR
  updated the support section
  removed the deprecated collision function from the previous 1.1.x version
  updated links in examples and added a 1.2.0 section in the Changelog
  bumped to 1.2.0 !
  ...
@ellisonleao
Copy link
Contributor Author

Not yet @obiot , js is really pissing me off with this one. I've tried rollbacking and use the old code with Q and shelljs but the error remains. I was thinking maybe we can use the shell script inside a grunt task to get this done, until we don't have fully synchronous builtin support in nodejs. What do you think @parasyte ?

@parasyte
Copy link
Collaborator

The sad truth is we cannot use shelljs until they fix shelljs/shelljs#51 ref. shelljs/shelljs#66

So ... I'll leave it up to you, where you want to take it from here.

@parasyte
Copy link
Collaborator

Or rather, we must use async shelljs calls, which is going to complicate the task unnecessarily. 😢

@ellisonleao
Copy link
Contributor Author

the combination using shelljs and Q promises is working 95% well, but it always getting stuck in the final task. I think we can create a simple grunt task using the release.sh script ( https://github.com/parasyte/hork/blob/master/tasks/release.sh) with some light modification to handle errors in the grunt task.

@parasyte
Copy link
Collaborator

@ellisonleao The only way to fix that hang is to run the commands async. See here: https://github.com/melonjs/melonJS/pull/528/files#diff-d95e7c916be2a81522fee0617e0d16a9R22

It needs to be rewritten to use the deferred promise, and all of the functions need to return the return value from run. E.g.:

function run(cmd, msg) {
    var deferred = Q.defer();
    shell.exec(cmd, shellOpts, function (code, output) {
        if (code === 0) {
            grunt.log.ok(msg || cmd);
            deferred.resolve();
        }
        else {
            // fail and stop execution of further tasks
            grunt.log.error("Error " + code + ": " + cmd);
            grunt.verbose.writeln(output);
            deferred.reject("Failed when executing '" + cmd  + "' \n");
        }
    });
    return deferred.promise;
}

function checkout() {
    // ...
    return run("git checkout --detach", "Detaching from current tree");
}

function rollback() {
    // ...
    return run("git reset --hard")
        .then(run("git checkout " + backBranch, "Resetting staged changes and back to initial branch"));
}

Also, notice rollback needs a slight modification, because it executes multiple commands...

@ellisonleao
Copy link
Contributor Author

nice approach. I will try this modifications and let you guys know.

@parasyte parasyte merged commit c14a1d6 into melonjs:master Jun 3, 2015
parasyte added a commit that referenced this pull request Jun 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants