This repository was archived by the owner on May 29, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9
#17 remove devdependencies #18
Open
xizhao
wants to merge
130
commits into
sourcegraph:master
Choose a base branch
from
xizhao:patch-6
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
130 commits
Select commit
Hold shift + click to select a range
4389499
Initial commit
sqs eaed470
Omit locals
sqs 33a6153
Print ref resolution stats
sqs bb9fb6d
Custom output format; use new tern fixes
sqs cacb68b
Set objectDef path to actual definition, not just location of reference
sqs f4d1d58
Ref annotation
sqs 9a899c2
ignore npm-debug.log
sqs 7e23c2e
Remove !s from key names
sqs 54cbcbb
exported
sqs 154d76e
More coherent, less opaque scheme for identifying symbols
sqs 87bb906
nodejs_require_mod0 test case
sqs 16484b4
Comment about types with no path
sqs c7723f6
Rm log
sqs a2b4634
Attach NPM package info to refs
sqs 739c969
Skip modules that fail to resolve to a type
sqs 3818d02
Fix originNode._path conflicts
sqs 7065371
Get sourceFile from child node if missing on parent node
sqs 41deddd
Resolve ecma5/browser origin type defs to their original path, not th…
sqs 6b7f8f0
Update node refs
sqs ef918bd
Revert to using logical node.js API module names
sqs 7ba296c
Determine correct ref targets for re-exported "node" origin types
sqs b0f35fd
Better nodejs origin types handling
sqs bc4cbda
Better handling of require and its properties
sqs 643e265
Protect against repeated changes
sqs 57274ac
Add node plugin test with coreModulesDir
sqs caafd33
Failing test for when a constructor overwrites a prototype method
sqs 9f4f4ef
resolution stats
sqs a855d81
Mark nodejsCoreModule on ref targets
sqs 3036d66
Fix module determination for node globals
sqs 164516f
add recv test case
sqs 96bec12
recv
sqs 12b2cc0
update TODO
sqs 28cb445
Prefer exported paths in tern-scope-walk
sqs 07630e9
Use func backprop, not method
sqs 150b521
parsed recv array
sqs 63b3bd3
get ref targets for recv
sqs 9f3a0c0
update
sqs d08c4ad
astNodeFilename
sqs 96bdfbe
Remove total views counter
sqs 156b62c
motivation and example
sqs 48a3892
symbols describer
sqs 263fb14
Escape unprintable chars in symbol paths
sqs f6ddadc
Add GraphKit logo
sqs af114b9
Add more images and center logo
sqs 9047bc8
postinstall script to remove other terns, allowing plugins to hook in…
sqs 46a36f9
Update exp to include additional info in recv
sqs b117d22
Revert "Update exp to include additional info in recv"
sqs 51bc6ca
update to latest tern (in sourcegraph branch)
sqs 95db9bf
bump deps
sqs 0683323
track definitions on ident nodes
sqs b33ede1
use ascii (to get byte offsets, not unicode character offsets)
sqs c7decbd
bump
sqs 1a47752
bump
sqs 4a951ac
revert to utf8 (doc comments are broken otherwise)
sqs 78a82f2
bump
sqs 90ea73e
initial commit from template
sqs 7545cb0
update license mention
sqs ff695d5
basic commonjs scanner using commonjs-findpkgs
sqs 2a8f76a
add test case javascript-nodejs-sample-0
sqs 25547dc
add jsg to node_modules/jsg
sqs 62cb576
port convertSymbol and convertRef from old JavaScript toolchain writt…
sqs fbff662
depresolve
sqs ce48025
depresolve and graph test exp
sqs 7903378
add javascript-nodejs-xrefs-0 test case
sqs 921a323
move old Go code to __old
sqs 287ce5f
normalize URIs; update javascript-nodejs-sample-0 to include docs; fi…
sqs b034cbe
set file
sqs 862397e
eliminate duplicate .js.js in ref symbolpath
sqs 1a888a7
remove superfluous lib/
sqs 09f7a97
increase maxBuffer
sqs 20d694c
s/name/unit/g (but the units are still wrong in docker)
sqs feadc42
logging
sqs 7720a0a
dockerignore
sqs 33c0421
npm install in docker container
sqs 796cb04
properly attribute xrefs to the right node_module for nodejs core mod…
sqs f754df3
update test Makefile exp output
sqs f76d13d
update exp (not sure why it lost the eql refs, but it kept the javasc…
sqs f909b87
bundle tern
sqs 7832c61
bundle all npm deps
sqs 2566e63
remove makefile
sqs 53e14cc
bundle all node_modules
sqs e135f73
Added missing tern bin from github.com/sqs/tern
andrewdeandrade 3f46354
Merge pull request #2 from malandrew/bug/gh-1
sqs 14d5f96
Remove npm install steps from Dockerfile now that all node_modules ar…
sqs 6ec3036
add both program and docker tests
sqs cb61d60
set up for travis CI
sqs bccf8f7
use src v0.0.2
sqs 5adff89
add toolchain in test
sqs e16ca88
use non-ssh git submodule URLs
sqs 1997441
fix sudo wget cmd
sqs 9b4df29
use src 0.0.4
sqs bba8236
set as default scanner
sqs ec63eef
use src 0.0.5 with automatic detection of scanners in srclibpath
sqs 3476d7f
more descriptive readme
sqs f320ecf
mention tern
sqs 0ecf9a9
add section about tests
sqs 06cffa7
justify
sqs 820e7fc
use src v0.0.7 (slightly different .srclib-cache layout)
sqs ab3f87c
use src v0.0.8 (Makefiles are not persisted in .srclib-cache)
sqs 8b41c7a
rename symbols -> defs
sqs 01a6903
expose def formatter
sqs cd0bebf
accept --config
sqs cc26bee
Revert "accept --config"
sqs ea89a22
upgrade commonjs-findpkgs to 0.0.7
sqs 2b81740
use src v0.0.12
sqs 2e249db
use --ignore-scripts in npm install to avoid pulling in unnecessary b…
sqs 002c976
install npm deps for tests
sqs 543a6c7
add minimal_nodejs_stdlib submodule
sqs 15a5d55
jsg plugins in source unit config, and test with stdlib
sqs eae999e
Don't include hardcoded abs path in unit expected output
sqs 510c2b4
use equinox.io binary url
sqs a6acaed
update Def fields (test exps)
sqs e28ecfd
ignore more pkgs (test/template)
sqs 17343ac
larger maxBuffer for jsg output
sqs f57355a
add some explanatory comments
beyang 7554607
Merge pull request #4 from sourcegraph/comments
beyang 29c423f
handle module.parent (fixes https://github.com/sourcegraph/srclib/iss…
sqs 956b949
Merge pull request #8 from sourcegraph/module-parent
sqs c23f5b6
support when package.json main is a dir (not file)
sqs 196be1c
support when package.json has no files
sqs aec81d8
js misc
sqs 18933fb
include-src-dir
sqs bba2543
include main dir
sqs ce5308d
omit syntax error pkgs
sqs 8a74ad9
test unicode
sqs e4e2122
add nodejs stdlib dep to all; also add Local field
sqs 6f2e41d
update srclib
sqs ade1ae5
omit bower_components dirs
sqs de2c161
local null -> false
sqs 73ff33a
#17 remove devdependencies
xizhao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
The diff you're trying to view is too large. We only load the first 3000 changed files.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,276 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| var parser = require("nomnom"); | ||
| var glob = require("glob"); | ||
| var findpkgs = require("commonjs-findpkgs"); | ||
| var jsgAdapter = require("../lib/jsg_adapter") | ||
| var npm = require("npm"); | ||
| var path = require("path"); | ||
| var execFile = require("child_process").execFile; | ||
| var fs = require("fs"); | ||
|
|
||
| parser.command("scan-commonjs") | ||
| .option("repo", { | ||
| help: "repository URI" | ||
| }) | ||
| .option("subdir", { | ||
| help: "subdirectory in repository" | ||
| }) | ||
| .callback(function(opts) { | ||
| // TODO(sqs): support ScanIgnore from repo Srcfile (pass to findpkgs ignore param) | ||
| findpkgs(".", ["node_modules"], function(err, pkgs) { | ||
| if (err) { | ||
| console.error("Scanning failed:", err); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| // omit package.json files that have syntax errors | ||
| pkgs = pkgs.filter(function(pkg) { return !pkg.error; }); | ||
|
|
||
| // filter out undesirable packages (packages whose paths contain | ||
| // "node_modules", "test", etc.) | ||
| pkgs = pkgs.filter(function(pkg) { | ||
| // TODO(sqs): we probably want to process things in node_modules for | ||
| // local code, so remove this filter. | ||
| var pathComps = pkg.dir.split("/"); | ||
| return pathComps.indexOf("node_modules") == -1 && pathComps.indexOf("test") == -1 && pathComps.indexOf("templates") == -1 && pathComps.indexOf("bower_components") == -1; | ||
| }); | ||
|
|
||
| // filter out undesirable source files (minified files) from | ||
| // packages | ||
| pkgs.forEach(function(pkg) { | ||
| pkg.libFiles = pkg.libFiles.filter(function(f) { | ||
| return !/\.min\.js$/.test(f); | ||
| }); | ||
| }); | ||
|
|
||
| // automatically include files in the src dir (not just | ||
| // lib). this is somewhat common for client-side libs even | ||
| // though it is technically in violation of the commonjs spec. | ||
| // TODO(sqs): add this functionality (as an option?) to commonjs-findpkgs? | ||
| pkgs.forEach(function(pkg) { | ||
| var srcFiles = glob.sync(path.join(pkg.dir, 'src/**/*.js')); | ||
| if (srcFiles) pkg.libFiles.push.apply(pkg.libFiles, srcFiles); | ||
| }); | ||
|
|
||
| // automatically include files in the dir containing the main file (if it's a subdir) | ||
| // TODO(sqs): add this functionality (as an option?) to commonjs-findpkgs? | ||
| pkgs.forEach(function(pkg) { | ||
| if (pkg.package.main && pkg.package.main.indexOf('/') != -1) { | ||
| var files = glob.sync(path.join(pkg.dir, path.dirname(pkg.package.main), '**/*.js')); | ||
| if (files) pkg.libFiles.push.apply(pkg.libFiles, files); | ||
| } | ||
| }); | ||
|
|
||
| // convert from commonjs-findpkgs format to source unit | ||
| var srcunits = pkgs.map(function(pkg) { | ||
| // collect all deps | ||
| var allDeps = [{name: "node", "version": ""}]; | ||
| function collectDeps(deps) { | ||
| if (deps) Object.keys(deps).forEach(function(name) { allDeps.push({name: name, version: deps[name]}); }); | ||
| } | ||
| collectDeps(pkg.package.dependencies); | ||
|
|
||
| files = arrayUnique(pkg.libFiles.concat(pkg.testFiles).filter(function(f) { return fs.statSync(f).isFile(); })); | ||
|
|
||
| return { | ||
| Name: pkg.package.name, | ||
| Type: "CommonJSPackage", | ||
| Dir: pkg.dir, | ||
| Files: files, | ||
| Dependencies: allDeps, | ||
| Data: pkg.package, | ||
| Ops: {depresolve: null, graph: null}, | ||
| Config: { | ||
| npmInstall: !!process.env.IN_DOCKER_CONTAINER, | ||
| jsg: { | ||
| plugins: { | ||
| node: { | ||
| coreModulesDir: process.env.NODEJS_CORE_MODULES_DIR || "$(JSG_DIR)/testdata/node_core_modules", | ||
| } | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| console.log(JSON.stringify(srcunits, null, 2)); | ||
| }); | ||
| }) | ||
| .help("scan for CommonJS packages"); | ||
|
|
||
| parser.command("depresolve") | ||
| .callback(function(opts) { | ||
| var stdin = process.stdin, | ||
| stdout = process.stdout, | ||
| inputChunks = []; | ||
|
|
||
| stdin.resume(); | ||
| stdin.setEncoding("utf8"); | ||
|
|
||
| stdin.on("data", function (chunk) { | ||
| inputChunks.push(chunk); | ||
| }); | ||
|
|
||
| stdin.on("end", function () { | ||
| var inputJSON = inputChunks.join(""); | ||
| var commonJSPackage = JSON.parse(inputJSON); | ||
|
|
||
| var remaining = commonJSPackage.Dependencies ? commonJSPackage.Dependencies.length : 0; | ||
| var resolutions = []; | ||
|
|
||
| if (remaining == 0) { | ||
| console.log('[]'); | ||
| return; | ||
| } | ||
|
|
||
| commonJSPackage.Dependencies.forEach(function(dep) { | ||
| var i = commonJSPackage.Dependencies.indexOf(dep); | ||
| resolutions[i] = {Raw: dep}; | ||
| var spec = dep.name+(dep.version ? "@"+dep.version : ""); | ||
| npm.load(function(err, npm) { | ||
| if (err) { | ||
| console.error("npm.load failed: ", err); | ||
| process.exit(1) | ||
| } | ||
|
|
||
| npm.commands.view(["--silent", spec], function(err,data) { | ||
| if (hardcodedRepoURLs[spec]) { | ||
| data = {"": {}} | ||
| err = null | ||
| } | ||
|
|
||
| if (err) { | ||
| resolutions[i].Error = "error occurred while resolving '" + spec + "': " + JSON.stringify(err); | ||
| } else if (!data || Object.keys(data).length == 0) { | ||
| resolutions[i].Error = "no npm package found with spec " + JSON.stringify(spec); | ||
| } else { | ||
| // Choose a version at random if it spits back a lot (TODO(sqs): | ||
| // this is stupid, we want the version that `npm install` would | ||
| // install, not just any random version, especially when | ||
| // version=='*'). | ||
| var chooseVersion = Object.keys(data)[0]; | ||
| var info = data[chooseVersion]; | ||
|
|
||
| var repoURL = ""; | ||
| if (hardcodedRepoURLs[spec]) { | ||
| repoURL = hardcodedRepoURLs[spec]; | ||
| } else if (info.repository) { | ||
| if (typeof info.repository == "string") repoURL = info.repository; | ||
| else repoURL = info.repository.url; | ||
| } | ||
|
|
||
| resolutions[i].Target = { | ||
| ToRepoCloneURL: repoURL, | ||
| ToUnit: info.name, | ||
| ToUnitType: "CommonJSPackage", | ||
| ToVersionString: info.version, | ||
| }; | ||
| } | ||
|
|
||
| remaining--; | ||
| if (remaining == 0) { | ||
| // done resolving all | ||
| console.log(JSON.stringify(resolutions, null, 2)); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| }); | ||
| }) | ||
| .help("resolves dependencies of a CommonJS package"); | ||
|
|
||
| var hardcodedRepoURLs = { | ||
| "node": "https://github.com/joyent/node.git", | ||
| }; | ||
|
|
||
| parser.command("graph") | ||
| .callback(function(opts) { | ||
| var stdin = process.stdin, | ||
| stdout = process.stdout, | ||
| inputChunks = []; | ||
|
|
||
| stdin.resume(); | ||
| stdin.setEncoding("utf8"); | ||
|
|
||
| stdin.on("data", function (chunk) { | ||
| inputChunks.push(chunk); | ||
| }); | ||
|
|
||
| stdin.on("end", function () { | ||
| var inputJSON = inputChunks.join(""); | ||
| var commonJSPackage = JSON.parse(inputJSON); | ||
|
|
||
| function graph() { | ||
| if (commonJSPackage.Files.length == 0) { | ||
| console.log("{}"); | ||
| process.exit(0); | ||
| } | ||
| jsgAdapter.run(".", commonJSPackage, function(err, graphData) { | ||
| if (err) { | ||
| console.error("Graphing " + commonJSPackage.Name + " failed:", err); | ||
| process.exit(1); | ||
| } | ||
| console.log(JSON.stringify(graphData, null, 2)); | ||
| }); | ||
| } | ||
|
|
||
| // TODO(sqs) install npm deps if we're in a docker container, since we can't rely on | ||
| // them to have been installed locally. we need to account for the fact | ||
| // that we've mounted the source volume readonly, so we can't just run | ||
| // `npm install`. | ||
| if (process.env.IN_DOCKER_CONTAINER && commonJSPackage.Config.npmInstall) { | ||
| // running `npm install` in the /src dir fails because it's mounted | ||
| // readonly. but `npm install -g` in the /src dir does nothing. so we | ||
| // need to `npm install /src` from a dir other than /src to get `npm | ||
| // install` to do anything. | ||
|
|
||
| var packageDir = path.join("/src", commonJSPackage.Dir); | ||
| console.error("Installing npm package in dir", packageDir, "..."); | ||
| execFile("npm", ["install", "--ignore-scripts", packageDir], {cwd: "/"}, function(err, stdout, stderr) { | ||
| if (stderr) console.error(stderr); | ||
| if (stdout) console.error(stdout); | ||
| if (err) { | ||
| console.error("warning: npm install in Docker container failed: ", err, "(continuing with graphing...)"); | ||
| } | ||
| graph(); | ||
| }); | ||
|
|
||
|
|
||
| // It's apparently impossible to silence npm install output when using | ||
| // npm.install. :( | ||
| // var oldcwd = process.cwd(); | ||
| // process.chdir(path.join(oldcwd, "..")); | ||
| // console.error("npm installing packages in Docker container..."); | ||
| // npm.load({loglevel: "silent"}, function(err, npm) { | ||
| // if (err) { | ||
| // console.error("npm.load in Docker container failed: ", err); | ||
| // process.exit(1) | ||
| // } | ||
| // // TODO(sqs): only works for top-level package.jsons. we actually need | ||
| // // "/src/${dirname(package.json path)", which also requires us to | ||
| // // track the path to the package.json. | ||
| // npm.commands.install(["/src"], function(err,data) { | ||
| // if (err) { | ||
| // console.error("npm install in Docker container failed: ", err); | ||
| // process.exit(1) | ||
| // } | ||
| // process.chdir(oldcwd); | ||
| // graph(); | ||
| // }); | ||
| // }); | ||
| } else { | ||
| graph(); | ||
| } | ||
| }); | ||
| }) | ||
| .help("graph a CommonJS package using jsg/tern"); | ||
|
|
||
| parser.parse(); | ||
|
|
||
| var arrayUnique = function(a) { | ||
| return a.reduce(function(p, c) { | ||
| if (p.indexOf(c) < 0) p.push(c); | ||
| return p; | ||
| }, []); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| .git | ||
| testdata | ||
| __old | ||
| .srclib-cache | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| testdata/actual/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| [submodule "testdata/case/javascript-nodejs-sample-0"] | ||
| path = testdata/case/javascript-nodejs-sample-0 | ||
| url = https://github.com/sgtest/javascript-nodejs-sample-0.git | ||
| [submodule "testdata/case/javascript-nodejs-xrefs-0"] | ||
| path = testdata/case/javascript-nodejs-xrefs-0 | ||
| url = https://github.com/sgtest/javascript-nodejs-xrefs-0.git | ||
| [submodule "testdata/case/minimal_nodejs_stdlib"] | ||
| path = testdata/case/minimal_nodejs_stdlib | ||
| url = https://github.com/sgtest/minimal_nodejs_stdlib.git | ||
| [submodule "testdata/case/js-misc"] | ||
| path = testdata/case/js-misc | ||
| url = https://github.com/sgtest/js-misc.git |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| language: node_js | ||
|
|
||
| node_js: | ||
| - "0.11" | ||
| - "0.10" | ||
|
|
||
| install: | ||
| - sudo wget -NO /tmp/src.zip 'https://api.equinox.io/1/Applications/ap_BQxVz1iWMxmjQnbVGd85V58qz6/Updates/Asset/src-0.0.22.zip?os=linux&arch=amd64&channel=stable' | ||
| - sudo unzip /tmp/src*.zip -d /usr/local/bin | ||
| - sudo mv /usr/local/bin/src-* /usr/local/bin/src | ||
| - sudo chmod +x /usr/local/bin/src | ||
| - src toolchain add sourcegraph.com/sourcegraph/srclib-javascript |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # How to Contribute | ||
|
|
||
| Sourcegraph projects are [BSD licensed](LICENSE) and accept contributions via | ||
| GitHub pull requests. This document outlines some of the conventions on | ||
| development workflow, commit message formatting, contact points and other | ||
| resources to make it easier to get your contribution accepted. | ||
|
|
||
| # Certificate of Origin | ||
|
|
||
| By contributing to this project you agree to the Developer Certificate of Origin | ||
| (DCO). This document was created by the Linux Kernel community and is a simple | ||
| statement that you, as a contributor, have the legal right to make the | ||
| contribution. See the [DCO](DCO) file for details. | ||
|
|
||
| # Email and Chat | ||
|
|
||
| The project currently uses the general Sourcegraph email list and IRC channel: | ||
| - Email: [srclib-dev](https://groups.google.com/forum/#!forum/srclib-dev) | ||
| - IRC: #[srclib](irc://irc.freenode.org:6667/#srclib) IRC channel on freenode.org | ||
|
|
||
| Please avoid emailing maintainers found in the MAINTAINERS file directly. They | ||
| are very busy and read the mailing lists. | ||
|
|
||
| ## Getting Started | ||
|
|
||
| - Fork the repository on GitHub | ||
| - Read the [README](README.md) for build and test instructions | ||
| - Play with the project, submit bugs, submit patches! | ||
|
|
||
| ## Contribution Flow | ||
|
|
||
| This is a rough outline of what a contributor's workflow looks like: | ||
|
|
||
| - Create a topic branch from where you want to base your work (usually master). | ||
| - Make commits of logical units. | ||
| - Make sure your commit messages are in the proper format (see below). | ||
| - Push your changes to a topic branch in your fork of the repository. | ||
| - Make sure the tests pass, and add any new tests as appropriate. | ||
| - Submit a pull request to the original repository. | ||
|
|
||
| Thanks for your contributions! | ||
|
|
||
| ### Format of the Commit Message | ||
|
|
||
| We follow a rough convention for commit messages that is designed to answer two | ||
| questions: what changed and why. The subject line should feature the what and | ||
| the body of the commit should describe the why. | ||
|
|
||
| ``` | ||
| scripts: add the test-cluster command | ||
|
|
||
| this uses tmux to setup a test cluster that you can easily kill and | ||
| start for debugging. | ||
|
|
||
| Fixes #38 | ||
| ``` | ||
|
|
||
| The format can be described more formally as follows: | ||
|
|
||
| ``` | ||
| <subsystem>: <what changed> | ||
| <BLANK LINE> | ||
| <why this change was made> | ||
| <BLANK LINE> | ||
| <footer> | ||
| ``` | ||
|
|
||
| The first line is the subject and should be no longer than 70 characters, the | ||
| second line is always blank, and other lines should be wrapped at 80 characters. | ||
| This allows the message to be easier to read on Sourcegraph and GitHub as well | ||
| as in various git tools. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now occurs to me that
devDependenciesinclude things like unit test libraries that it is useful to include as an "actual dependency." I think this was the original motivation behind including them in the list of deps. Is their presence causing major issues?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's rare especially for code you depend on -- you might have a point for code in your own environment. But w/ deps at least you're not going to be distributing tests or using them in actual code.
I think the npm convention is deps are for distribution and devDeps aren't, and you should only receive production-ready releases. Note the NPM install
--productionflag: https://docs.npmjs.com/cli/install, it seems like if people are usingdevDependenciesto include "actual dependencies", they're not usingdevDependenciescorrectly.So if you're depending on a package, you don't care about their dev-deps, only their production-ready release. Maybe there's a way we can provide a flag to srclib-javascript, so that if it's a local root project, we care about devDeps, but otherwise we ignore?
Also: what are your thoughts on peerdeps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default
npm installbehavior: