Bugzilla – Full Text Bug Listing
| Summary: | node.js implementation of our git pages using http://www.nodegit.org/ | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Paul Webster <pwebster> |
| Component: | Node | Assignee: | Albert Cui <aqc2109> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | aqc2109, bleathem, brandonadalesandro, elefthei, gheorghe, mamacdon, mt2837, simon_kaegi |
| Version: | 8.0 | ||
| Target Milestone: | 10.0 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=474353 | ||
| Whiteboard: | |||
|
Description
Paul Webster
Example command I use to run node: node server.js -dev -log -c $HOME/orion/nodejs/orion.conf Example orion.conf I use to run the node server: ## See http://wiki.eclipse.org/Orion/Node/Getting_started#The_orion.conf_file ## Path to npm-cli.js on your computer npm_path=/opt/local/node-v0.10.35-linux-x64/lib/node_modules/npm/bin/npm-cli.js ## Port number for the node inspector. When node inspector process starts, the "--web-port" argument is assigned this value. Default is 8900 if not defined here. #node_inspector_port=8900 ## Restrict access to the server using basic HTTP authentication. pwd=SomePassword # The "workspace" serves as the root for editing files and folders in Orion. workspace=/opt/users/pwebster/orion/nodejs/workspace Suggestion for collaborative github work. 1) name one repo (forked from eclipse/orion.client) as the canonical repo for your project. 2) give all team members write access on that repo 3) pick a branch to do your feature work 4) have everybody clone that one repo on their computers (i.e. don't fork it on github). 5) make sure you set branch.<feature_branch>.rebase=true on your local repos. 6) use git pull and git rebase and don't use git merge. PW The git workflow looks good to me. The only thing we have to do is decide who hosts the central repo and re-setup using that repo. When making changes to the pageLinksPlugin.js you have to open a web-console for your browser page and use localStorage.clear(). Then reload the page. PW # be checked out in the correct branch git fetch origin git reset --hard origin/node_git_pages Link to repo and branch: https://github.com/albertcui/orion.client/tree/node_git_pages Branch: node_git_pages Just a reminder that you should be adding tests in the org.eclipse.orion.client/modules/orionode/test, and running them regularly with npm test (in the org.eclipse.orion.client/modules/orionode directory). PW Please make sure that everybody sets the branch.node_git_pages.rebase = true, so that pull won't create any merge commits. Also, you can use git config --global branch.autosetuprebase true Then any tracking branches that are set up would get the rebase config property automatically. PW Is it possible to run these through the node server? http://localhost:8000/js-tests/webtools/webtoolsMochaTests.html http://localhost:8000/js-tests/ui/uiTests.html http://localhost:8000/js-tests/core/coreTests.html http://localhost:8000/js-tests/javascript/JsMochaSuite.html PW Just a quick written update: 1) Refactored all the code in orionode/lib/git.js to respective js files in orionode/lib/git. 2) We have git clone and git remotes working. 3) Currently working on git status. 4) Git branches and git config we'll deal with later, since it looks like the devs of nodegit are pushing out new changes as we speak (we're now using a "working" version of nodegit, not a stable one.) Questions: What are files in git that are "changed", or what's the difference between files that are "modified" vs. "changed"? (For reference: https://wiki.eclipse.org/Orion/Server_API/Git_API#Getting_status_for_a_git_project.) We've Googled but have the same questions about files marked as "missing" or "removed". Please let us know if you have any idea! Take a look at gitChangeList.js You will see that unstaged files can be in one of the following states: [missing, modified, untracked, confliciting] Staged files can be: [added, changed, removed] The module ./git/index seems to be missing from the last commit. Removed the require. The file was created then deleted, but we forgot to remove the require. Sorry about that! I think there's something fishy going on with the Orion Git API, https://wiki.eclipse.org/Orion/Server_API/Git_API#Initializing_a_git_repository. It shows git clone and git init as having the same path (/gitapi/git/clone) but in reality, the front-end sends POST requests to /gitapi/git/clone for clone and POST /gitapi/clone for git init. Update:
You can now stage one file/multiple files. If you want to stage one file, you can make one call via a URL. If you want to stage multiple files, you send json data with an array under the "Path" key. Ex: {"Path": ["file1.txt", "file2.txt", ...] }
Another update:
Git add components:
stage a file: (done => only tested with spoofed data)
PUT /gitapi/index/file/A/folder/file.txt
(or)
PUT /gitapi/index/file/A/
data: {"Path": [list, of, files]}
unstage a file: (done => only tested with spoofed data)
PUT /gitapi/index/file/E/folder/file.txt
(or)
PUT /gitapi/index/file/E/
data: {"Path": [list, of, files]}
get info from index: (IP)
GET /git/index/file/MyProj/file.txt
Gets info about the file in the index.
https://docs.google.com/spreadsheets/d/13lF7ORXufhkAsYBKB1eCgNfP2eb7szmVq-HhZJJagLA/edit#gid=1115712044 I updated the Google Doc (under the tab Task Log), which should have all the methods we're supposed to implement. We're still filling it out right now, but Paul, this should give you a better idea on what's done and who did what. Hi, nodegit 0.2.7 finally installed on my RHEL system :-) but you mentioned you might be using the latest. How do you download/install it so that running our node server can use that one? Assume that I cloned it from https://github.com/nodegit/nodegit PW Sorry I didn't get to it today, but I'll have a look tomorrow PW I got node to install nodegit 0.2.7, and pulled the latest changes on the node_git_pages branch.
When I fire up nodejs and go to /git/git-repository.html page, I get an "Unknown error has occurred" and the following stack trace.
GET /workspace/orionode 200 - - 5 ms
Error: invalid json
at Object.exports.error (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/lib/utils.js:44:13)
at IncomingMessage.<anonymous> (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/lib/middleware/json.js:68:73)
at IncomingMessage.emit (events.js:117:20)
at Object.resume (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/node_modules/pause/index.js:25:18)
at resume (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/lib/middleware/static.js:61:13)
at SendStream.error (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/lib/middleware/static.js:73:37)
at SendStream.emit (events.js:95:17)
at SendStream.error (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/node_modules/send/lib/send.js:144:51)
at SendStream.onStatError (/home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/node_modules/send/lib/send.js:245:48)
at /home/pwebster/git/nodegit.orion.client/modules/orionode/node_modules/connect/node_modules/send/lib/send.js:317:26
at Object.oncomplete (fs.js:108:15)
I can't see the the buttons for creating a git repo (Either Init or Clone) and I can't see any repos listed even though I manually cloned a repo into the workspace/project location.
I ran npm test and there aren't any git tests in the output.
I'll see if I can give any feedback on the code in another comment.
PW
Paul, Did you npm install to update connect? It looks like it's the issue we found with request body parsing. -Albert (In reply to Albert Cui from comment #22) > Paul, > > Did you npm install to update connect? It looks like it's the issue we found > with request body parsing. I had a clean repo and did a full npm install. It installed connect 2.4.6. PW Code feedback comments:
It seems like findit, mkdirp, and path could probably be removed to keep the dependencies down.
modules/orionode/lib/git.js
Do you need this line?
var git = require('nodegit');
This is not needed in most of your modules:
var finder = require('findit');
modules/orionode/lib/git/add.js
If you're just joining to strings, I don't think you need the "path" module, do you?
modules/orionode/lib/git/branches.js
Don't need finder or path.
Make a note somewhere that this needs to be fixed:
"LocalTimeStamp": 1424471958000, //hardcoded local timestamp
This module seems to be using the files directly instead of using the nodegit API. It seems like either Refs or Branch could be used to get the list of branches from the repo.
Although commented out, checkRemote(*) should be using the nodegit Remote API.
modules/orionode/lib/git/clone.js
findit module: Why use findit instead of just using the standard node fs module?
mkdirp module: you probably don't need this, you can write a small function to do what you want.
modules/orionode/lib/git/config.js
If the ini module parses the config file correctly, I think we have to keep using it for reading properties until nodegit supports reading properties.
putConfig(*): you should use the nodegit Config API to write to the config file, since it's potentially destructive and the API exists.
modules/orionode/lib/git/tags.js
Once you have the string name, you can use Refs.lookup(*) to get the Reference object and then use isTag() to see if it's a tag.
modules/orionode/lib/orionode.client/plugins/pageLinksPlugin.js
Please add the code so that you can get to the git page from the side bar.
Thanks a lot, folks.
PW
Paul, I think it should be installing 2.8. See here: https://github.com/albertcui/orion.client/blob/node_git_pages/modules/orionode/package.json#L8 -Albert (In reply to Albert Cui from comment #25) > Paul, > > I think it should be installing 2.8. See here: OK, I'll try and bump it up. PW (In reply to Albert Cui from comment #22) > Paul, > > Did you npm install to update connect? It looks like it's the issue we found > with request body parsing. I've fixed this issue but I can't do anything with nodegit. With that github URL it doesn't install correctly. It downloads an index.js but no package.json which is why it yacks. npm http 200 https://github.com/nodegit/nodegit.git npm ERR! not a package /home/pwebster/tmp/npm-24799-5hFemU1r/1426257551370-0.8312433739192784/tmp.tgz npm ERR! Error: ENOENT, open '/home/pwebster/tmp/npm-24799-5hFemU1r/1426257551370-0.8312433739192784/package/package.json' npm ERR! If you need help, you may report this log at: npm ERR! <http://github.com/isaacs/npm/issues> npm ERR! or email it to: npm ERR! <npm-@googlegroups.com> npm ERR! System Linux 3.17.4-301.fc21.x86_64 npm ERR! command "node" "/usr/bin/npm" "install" npm ERR! cwd /home/pwebster/git/nodegit.orion.client/modules/orionode npm ERR! node -v v0.10.36 npm ERR! npm -v 1.3.6 npm ERR! path /home/pwebster/tmp/npm-24799-5hFemU1r/1426257551370-0.8312433739192784/package/package.json npm ERR! code ENOENT npm ERR! errno 34 npm ERR! npm ERR! Additional logging details can be found in: npm ERR! /home/pwebster/git/nodegit.orion.client/modules/orionode/npm-debug.log npm ERR! not ok code 0 (In reply to Paul Webster from comment #27) > > I've fixed this issue but I can't do anything with nodegit. With that > github URL it doesn't install correctly. It downloads an index.js but no > package.json which is why it yacks. > The package.json has to start with git+https instead of just https. Could someone fix that in the branch please? ex: "nodegit": "git+https://github.com/nodegit/nodegit.git", PW (In reply to Paul Webster from comment #21) > I ran npm test and there aren't any git tests in the output. > OK, I can now see the 2 git tests (one of which is failing :-) PW (In reply to Paul Webster from comment #21) > > I can't see the the buttons for creating a git repo (Either Init or Clone) > and I can't see any repos listed even though I manually cloned a repo into > the workspace/project location. Now that I've fixed the nodegit in package.json I see the buttons and I can see the repo I have already cloned into the workspace. When I go to clone another repo, https://github.com/nodegit/nodegit.git, and click "Submit" on the node server I see: POST /login 404 - - 1ms and nothing else happens. PW It seems like findit, mkdirp, and path could probably be removed to keep the dependencies down. > Path is a standard Node library function. We got rid of findit on all files except clone.js. -------------------------------------------------------- modules/orionode/lib/git.js Do you need this line? var git = require('nodegit'); > Removed from places it's not used (including git.js)! This is not needed in most of your modules: var finder = require('findit'); > Removed from places it's not used (including git.js)! -------------------------------------------------------- modules/orionode/lib/git/add.js If you're just joining to strings, I don't think you need the "path" module, do you? > We used this because it automatically joins strings with a slash and takes care of certain edge cases (https://nodejs.org/api/path.html#path_path_join_path1_path2). > Path is a standard Node library function. Can we leave it in? -------------------------------------------------------- modules/orionode/lib/git/branches.js Don't need finder or path. Make a note somewhere that this needs to be fixed: "LocalTimeStamp": 1424471958000, //hardcoded local timestamp This module seems to be using the files directly instead of using the nodegit API. It seems like either Refs or Branch could be used to get the list of branches from the repo. Although commented out, checkRemote(*) should be using the nodegit Remote API. > WIP -------------------------------------------------------- modules/orionode/lib/git/clone.js findit module: Why use findit instead of just using the standard node fs module? > The thing with this is, if we use fs, the code gets very messy. We'd have to check each repo to see if it's valid, check for folders within that repo, and recursively call the function on each folder. > findit allows us to do in a better way because it's event based. We can change it if you'd like, but the code definitely looks cleaner as is. mkdirp module: you probably don't need this, you can write a small function to do what you want. > Removed (it wasn't being used anyway)! -------------------------------------------------------- modules/orionode/lib/git/config.js If the ini module parses the config file correctly, I think we have to keep using it for reading properties until nodegit supports reading properties. putConfig(*): you should use the nodegit Config API to write to the config file, since it's potentially destructive and the API exists. > Done! -------------------------------------------------------- modules/orionode/lib/git/tags.js Once you have the string name, you can use Refs.lookup(*) to get the Reference object and then use isTag() to see if it's a tag. > Fixed! -------------------------------------------------------- modules/orionode/lib/orionode.client/plugins/pageLinksPlugin.js Please add the code so that you can get to the git page from the side bar. > Pretty sure this already works - we've been using it. Did you do that console command that clears window history/cache? -------------------------------------------------------- Paul: For DELETE config, there's nothing on nodegit right now that'll allow us to delete an reset an option in the config file. Can we just use ini or should we move onto something else? (In reply to Mei-Vern Then from comment #31) > modules/orionode/lib/git/add.js > > If you're just joining to strings, I don't think you need the "path" module, > do you? > > We used this because it automatically joins strings with a slash and takes care of certain edge cases (https://nodejs.org/api/path.html#path_path_join_path1_path2). > > Path is a standard Node library function. Can we leave it in? Yes, if it's standard in node.js then I think it's OK. > > modules/orionode/lib/git/clone.js > > findit module: Why use findit instead of just using the standard node fs > module? > > The thing with this is, if we use fs, the code gets very messy. We'd have to check each repo to see if it's valid, check for folders within that repo, and recursively call the function on each folder. > > findit allows us to do in a better way because it's event based. We can change it if you'd like, but the code definitely looks cleaner as is. > I'm still on the fence about this, since walking a file tree isn't really something you need event based complex searches for. But why don't you leave it in for now, and if we need to get rid of it later we can write an object to encapsulate it. > -------------------------------------------------------- > > modules/orionode/lib/orionode.client/plugins/pageLinksPlugin.js > > Please add the code so that you can get to the git page from the side bar. > > Pretty sure this already works - we've been using it. Did you do that console command that clears window history/cache? > -------------------------------------------------------- I'll double check the next time I fire it up, maybe I just need to clear my cache. Thanks for the update. PW (In reply to Mei-Vern Then from comment #32) > Paul: For DELETE config, there's nothing on nodegit right now that'll allow > us to delete an reset an option in the config file. Can we just use ini or > should we move onto something else? I'd move onto something else. For now, deleting a piece of config is lower priority. Later, Paul Just a note on the usecase that should focus which parts of the code you work on: 1) clone a public repo on the Git page 2) edit a couple of files 3) switch back to the Git page 4) select some of the files 5) create a commit 6) push to the public repo The second one is the same as the first, except #1 is fetch+rebase IIRC the clone step doesn't work correctly. That should be the next highest priority. PW I updated the version of pty.js to 0.2.5, as we were having issues installing on OSX 10.10. I also changed nodegit to use 0.3.3 so that we're using the stable version. I believe it has all the features we need for now. However, I've been having issues installing nodegit. Running npm install will install it, but when I start the server, I get a require issue. Something about module nodegit not found. Building from source (cd in to node_modules first) seems to fix the issue: http://www.nodegit.org/guides/install/from-source/ I've notified the nodegit people of this issue. Hopefully it gets fixed? Quick review comments: 1. there is no Git settings section in the "Settings" page. It should be added. 2. git clone gets a 404 asking for /login, so you can't clone from the page 3. After I cloned https://github.com/albertcui/orion.client.git manually into the workspace the git page found the orion.client, but then I got an unknown error (a timeout on the GET for http://172.17.0.11:8081/gitapi/status/file/orion.client ) Lef is looking at #2, AFAIK. Albert, Mei-Vern, your doc says you looked at status could you take a look at #3? Also, it's important to clean up these bugs quickly, since we will need to do a test pass on the code this week/weekend. The primary usecases need to work so you can start fixing some of the secondary paths (i.e. what happens when you try and clone a repo that's broken, or that's not there, and does it match the main version). Thanks Folks, Paul Re status.js var statuses = repo.getStatusExt(); returns a Promise statuses.forEach(*) throws an exception: TypeError: Object #<Promise> has no method 'forEach' PW Paul: We're not having this issue. Are you running node 0.10.36+? (In reply to Albert Cui from comment #39) > Paul: We're not having this issue. Are you running node 0.10.36+? Yes, I'm running node:0.10.38 I completely removed node_modules in orionode and did a npm install PW my node_modules/nodegit/package.json say: "name": "nodegit", "description": "Node.js libgit2 asynchronous native bindings", "version": "0.3.3", PW I was having issues using npm install to install nodegit actually. Can you try building from source in the nodegit directory in node_modules? http://www.nodegit.org/guides/install/from-source/ (In reply to Albert Cui from comment #39) > Paul: We're not having this issue. Are you running node 0.10.36+? When you debug, what does var statuses = repo.getStatusExt(); return? A Promise object, or something else? PW (In reply to Albert Cui from comment #42) > I was having issues using npm install to install nodegit actually. > > Can you try building from source in the nodegit directory in node_modules? > > http://www.nodegit.org/guides/install/from-source/ I thought we used to have the correct thing in the package.json to pick up nodegit from its repo instead of from npm? PW For us it returns an array. I changed package.json to use npm instead because I figured we should be on a stable build. I can change it back to use github instead? (In reply to Albert Cui from comment #45) > For us it returns an array. > > I changed package.json to use npm instead because I figured we should be on > a stable build. I can change it back to use github instead? Sure, if that updates the API so it's consistent. I'll try it out after the update. I wonder if it returns a Promise if it thinks it's going to take some time? PW (In reply to Paul Webster from comment #46) > (In reply to Albert Cui from comment #45) > > For us it returns an array. > > > > I changed package.json to use npm instead because I figured we should be on > > a stable build. I can change it back to use github instead? > > Sure, if that updates the API so it's consistent. I'll try it out after the > update. I wonder if it returns a Promise if it thinks it's going to take > some time? > > PW The nodegit api says it's a synchronous function. I have some experience with nodegit. Is there anywhere I can get involved in the implementation of this feature? Is there a git branch published somewhere with any code written so far? (In reply to Brian Leathem from comment #48) > I have some experience with nodegit. Is there anywhere I can get involved > in the implementation of this feature? > > Is there a git branch published somewhere with any code written so far? If you want to look at what's available so far, check out https://github.com/albertcui/orion.client/tree/node_git_pages PW Great, thanks. I was able to build and run the node_git_pages branch locally. I like the focus of working towards a fundamental use case. Is there a breakdown anywhere of the work remaining to achieve that goal? (In reply to Brian Leathem from comment #50) > Great, thanks. I was able to build and run the node_git_pages branch > locally. > > I like the focus of working towards a fundamental use case. Is there a > breakdown anywhere of the work remaining to achieve that goal? We can currently initialize a repo, add a file, and commit it. We have not tested adding a remote, and I believe pushing to a remote does not work right now. We would like to be able to clone a repository, but that is currently not working either. I will be adding a test to verify that the current init>add>commit pipeline is working this weekend. (In reply to Paul Webster from comment #38) > Re status.js > > var statuses = repo.getStatusExt(); returns a Promise > > statuses.forEach(*) throws an exception: > TypeError: Object #<Promise> has no method 'forEach' > > PW I still get this problem when running firefox as the client, node as the server, and node-inspector against google-chrome to debug. Set the breakpoint in status.js:25 and step over and statuses is a Promise object. PW If I modify status.js to treat it like a promise:
statuses.then(function(sarray) {
sarray.forEach(function(file) {
...
res.end(resp);
});
})
.catch(function(err) {
then it actually works in my setup :-)
I'm on node 0.12.2
PW
Ok, I'll patch that asap Good job on getting clone to kick off. It reports an error because orionode doesn't implement Task/Progress, but that's probably out of scope for what you guys are doing. The clone itself is still running after the client says it's failed with an error. It looks like I still have that weird difference in getStatusExt(*) returns a Promise in my environment. But I'm using the vanilla docker node image. With a workaround I can change files and see the changed files on the git page. I modifed the README.md file in orion.client that's checked out in my node workspace, and it shows up under working directory changes. I selected the files and created a bogus commit. It committed an empty commit. When I selected the changed file a second time, the second commit did contain the files. Not sure what's going on here, but the commit worked. Not sure if I can push yet. Other comments: The REFERENCE: field say master => origin/master [New branch], but it's the default master tracking configuration and should just say master => origin/master. All of the commits are listed in the "Outgoing" section instead of in the "History" section. This is possibly related to the same data that's allowing [New Branch] to show up. If I go to REFERNCES: and expand them, go to local>master and try and checkout I get "Problem while performing the action". If I select all of my changed files and try the "checkout selected files, discarding all changes" which looks like a trash can, I get the same error message. Looks like I can't get rid of modified files. When I expand REFERENCES and go to origin node_git_pages there are no icons on that line that would allow me to check out that branch. The usual behaviour is checking out a branch like origin/node_git_pages should create the local, so you end up with node_git_pages => origin/node_git_pages. There are a lot of things to continue to work on, but I do want to say that this is some really good work to date. Thanks folks, Paul (In reply to Brandon Dalesandro from comment #54) > Ok, I'll patch that asap Thanks Brandon, If this is working in everybody elses environment, you might have to check to see if you get back an array (and process it now) or a promise (and process it later). PW Ok I'll look into that. It looks like it's a promise though. Brandon, looks like we'll have to adapt to both. In 0.3.3 it returns a Promise, https://github.com/nodegit/nodegit/blob/v0.3.3/lib/repository.js#L775 In the next revision, it'll return an Array. https://github.com/nodegit/nodegit/blob/master/lib/repository.js#L788 PW (In reply to Paul Webster from comment #58) > Brandon, looks like we'll have to adapt to both. In 0.3.3 it returns a > Promise, > https://github.com/nodegit/nodegit/blob/v0.3.3/lib/repository.js#L775 > > In the next revision, it'll return an Array. > https://github.com/nodegit/nodegit/blob/master/lib/repository.js#L788 > > PW Ah!!! That explains why it works for us. We built from source for our nodegit installation. (In reply to Paul Webster from comment #55) > Good job on getting clone to kick off. It reports an error because orionode > doesn't implement Task/Progress, but that's probably out of scope for what > you guys are doing. The clone itself is still running after the client says > it's failed with an error. I actually did a basic implementation of task in lib/tasks.js. I don't know if it's right, but we couldn't find any documentation so it's kind of reverse engineered from what the Java server was sending. Push is using it. I didn't have time to get clone to use it. (In reply to Paul Webster from comment #55) > If I go to REFERNCES: and expand them, go to local>master and try and > checkout I get "Problem while performing the action". If I select all of my > changed files and try the "checkout selected files, discarding all changes" > which looks like a trash can, I get the same error message. Looks like I > can't get rid of modified files. > > When I expand REFERENCES and go to origin node_git_pages there are no icons > on that line that would allow me to check out that branch. The usual > behaviour is checking out a branch like origin/node_git_pages should create > the local, so you end up with node_git_pages => origin/node_git_pages. None of the checkout routes have been done. Code review comments: Most of the warnings and errors are easy to see when editing in Orion. These need to be cleaned up so we can accept the code into Orion. lib/git.js line: 14 - there are a couple of require(*)s that are assigned to variables that are never used. Can they please be removed if possible? line 39: - says to throw an Error instead line 138: - you can use the rmdir from the beginning of the file, instead of loading it during a REST call. lib/git/add.js line 14: - more unused requires line 17: - it says getFileIndex has unused parameters. Can they be removed? - also doesn't look like var result is used. line 41: - missing semi-colon There are a number of other warnings in this file that should be addressed (unused parameters, found '==' instead of '===', unused vars, and missing semi-colons). lib/git/blame.js - similar to above, unused requires, unused parameters in getBlame (but they're OK in the function passed to on(*)). lib/git/branches.js - There are unused requires, unused vars, and missing semi-colons. - there are also a few JS errors around line 135-159 that need to be addressed lib/git/clone.js - fix up unused vars and missing semi-colons. - Error on line 22 lib/git/commit.js Check for unused requires, unused var, and missing semi colons. There are JS errors around line 106 that need to be fixed. Also around line 302. lib/git/config.js Check for unused requires, unused var, and missing semi colons. lib/git/diff.js Check for unused requires, unused var, and missing semi colons. There are JS errors around line 51. lib/git/remotes.js Check for unused requires, unused var, and missing semi colons. line 226: missing parentheses when invoking a constructor lib/git/stash.js Check for unused requires, unused var, and missing semi colons. line 29: git.Repository.open(workspaceDir+"/booom") <- why is booom hard coded here. lib/git/status.js Check for unused requires, unused var, and missing semi colons. line 57: this needs to be updated to check if statuses is a Promise or an Array and execute accordingly. lib/git/tags.js Check for unused requires, unused var, and missing semi colons. lib/tasks.js Check for unused requires, unused var, and missing semi colons. line 35: there's a JS error test/test-git-api.js Check for unused requires, unused var, and missing semi colons. I've pushed the node_git_pages branch to git.eclipse.org on branch pwebster/bug458883 If you want to take a crack at fixing the last review comments, please submit a Gerrit review against the URLs listed in https://git.eclipse.org/r/#/admin/projects/orion/org.eclipse.orion.client PW I merged the branch to master, with some additional changes on top to address the warnings mentioned in Comment 63. Git doesn't permit multiple authors, so I used `Also-by:` tags to to indicate shared authorship, which is apparently the Eclipse way of doing this. Thanks for all your hard work on this, team. |