Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Add git-salt lib and git script #242

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

gaurave
Copy link
Contributor

@gaurave gaurave commented Apr 27, 2015

  • Adds git-salt library to axiom
  • Adds a git script to axiom which should act like git command line.

TODO

  • Add parsing in git script and route the commands to appropriate API.
  • Re-factor code into classes
  • Route stdio and stderr correctly.

@rpaquay

@ussuri
Copy link
Contributor

ussuri commented Apr 28, 2015

Just a general observation, not regarding this PR: the scripts directory's name is very confusing. We should rename it to something clearer.

@gaurave
Copy link
Contributor Author

gaurave commented Apr 28, 2015

@ussuri yes it is confusing, Ideally they should not be part of repository and should be hosted elsewhere. We should rename them may be external_scripts / addon_scripts / foreign_scripts

@@ -55,6 +55,10 @@ module.exports = function(grunt) {

mainsrc += '</head>\n';
mainsrc += '<body>\n';

// Div where addons embedding code live.
mainsrc += '<div id="addons"></div>';
Copy link
Contributor

Choose a reason for hiding this comment

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

This task is supposed to be generic, so we should not have this here. Can't we create the 'div' manually if it does not exist (in the code that creates the sub-elements)?

@gaurave
Copy link
Contributor Author

gaurave commented Apr 30, 2015

@rpaquay PTAL thanks.

});

function genMessageId() {
messageId++;
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation?

/**
* Remove the NaCl module from the page.
*/
GitSalt.prototype.removeModule = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any call to this function, does this mean we leak a git object at each invocation?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants