Commit Graph

731 Commits

Author SHA1 Message Date
Dandelion Mané 417265a4d4
Add `MapUtil.merge` for combining maps (#794)
See the docstring for details.

Test plan: Unit tests.
2018-09-06 14:53:49 -07:00
William Chargin 513820c177
deps: upgrade `flow-bin@^0.80.0` (#791)
Summary:
This upgrade didn’t require fixing any new errors, but Flow is a good
dependency to keep on top of.

Test Plan:
Running `yarn flow` suffices.

wchargin-branch: flow-v0.80.0
2018-09-06 13:37:29 -07:00
William Chargin 0b4fea1c4f
flow: add typing to Jest transform modules (#790)
Test Plan:
`yarn flow` passes, and the [Jest docs][1] suggest that the added type
annotations are correct.

[1]: https://jestjs.io/docs/en/configuration#transform-object-string-string

wchargin-branch: flow-jest-transforms
2018-09-06 13:30:33 -07:00
William Chargin 92514ad559
deps: remove some unused packages (#789)
Summary:
Mostly Webpack loaders that have become unused through various config
changes.

Test Plan:
Check that these packages are not used anywhere except as transitive
dependencies:

```shell
$ git show --format= package.json |
>     sed '1,4d' | grep '^-' | cut -d\" -f2 | git grep -cf -
yarn.lock:3
```

Also, `yarn && yarn test --full` works, and `yarn start` works, and
`yarn backend && node ./bin/sourcecred.js load sourcecred/example-git`
works.

wchargin-branch: remove-unused-deps
2018-09-06 12:08:45 -07:00
Dandelion Mané c2920547c0 Refactor PluginWeightConfig (#788)
This refactors PluginWeightConfig so that it uses the
`defaultWeightsForAdapter` method introduced in #787.

The refactor is mildly invasive, as we switch the state from being a
(mutable) `WeightedTypes` to having a (regular, read-only)
`WeightedTypes`. I think this is an improvement in consistency.

Test plan: Trivial refactor; unit tests+flow pass.
2018-09-05 17:39:21 -07:00
Dandelion Mané b40c1d078d
Create a `weights` module with types and utils (#787)
This commit creates a central `weights` module that defines all of the
weight-related types, and provides some utilities for dealing with them.

This way users of weight-concepts do not need to depend on a lot of
random modules just to get the relevant types. The utility methods are
implicitly defined a few places in the codebase: now we can avoid
re-writing them, and test them more thoroughly.

Test plan: Unit tests pass.
2018-09-05 17:34:29 -07:00
Dandelion Mané d77c76082d
credExplorer gets adapters from one point (#786)
Currently, the `credExplorer` uses the `defaultStaticAdapters`, but it
imports these adapters in multiple places. If we decide to make the
adapters configurable (e.g. when we start supporting more plugins) this
will be a problem.

This change modifies the cred explorer so that the adapters always come
from a prop declaration on the app. Then the adapters are passed into
the `state` module's functional entry points, rather than letting
`state` depend on the default adapters directly.

This change is motivated by the fact that my WeightConfig cleanup can be
done more cleanly if the adapters are present as a prop on the App.

Test plan: Unit tests are updated. Also, `git grep
defaultStaticAdapters` reaveals that the adapters are only consumed
once.
2018-09-05 16:10:11 -07:00
Dandelion Mané f7383bbc90
Add `weightsToEdgeEvaluator` (#785)
This commit adds `weightsToEdgeEvaluator`, a function for converting
weighted node types into an `EdgeEvaluator`. This replaces the
`edgeWeights` module (which was untested, and an outmoded API).

Test plan: The new `weightsToEdgeEvaluator` method is well-tested.
Since `WeightConfig` is still not tested, I manually verified that it
still works as anticipated.
2018-09-05 13:40:40 -07:00
William Chargin 3dda4ab35c
test: invoke `yarn backend` only once (#784)
Summary:
Lots of tests need the output of `yarn backend`. Before this commit,
they tended to create it themselves. This was slow and wasteful, and
also could in principle have race conditions (though in practice usually
tended not to).

This commit updates tests to respect a `SOURCECRED_BIN` environment
variable indicating the path to an existing directory of backend
applications.

Closes #765.

Test Plan:
Running `yarn test --full` passes.

Prepending `echo run >>/tmp/log &&` to the `backend` script in
`package.json` and running `yarn test --full` results in a log file
containing only one line, indicating that the script really is run only
once.

wchargin-branch: deduplicate-backend
2018-09-05 12:47:54 -07:00
Dandelion Mané bd7b7dec82
Export fallback adapter's node and edge types (#783)
This is convenient for testing other code, where we may want to directly
use the fallback types. One test has been updated in this way.

I also changed the names for the fallback adapter's edges to be somewhat
more readable.

Test plan: Tests improved.
2018-09-05 12:34:32 -07:00
William Chargin c48597651e
backend: invoke Webpack directly in package script (#782)
Summary:
This commit removes the `config/backend.js` script and replaces it with
a direct invocation of Webpack. This enables us to use command-line
arguments to Webpack, like `--output-path`.

Test Plan:
Note that `rm -rf bin; yarn backend` still works, and that the resulting
applications work (`node bin/sourcecred.js load`). Note that `yarn test`
and `yarn test --full` still work.

wchargin-branch: backend-webpack-direct
2018-09-05 12:28:27 -07:00
William Chargin ff2cfd14b2
backend: set Babel flags in Webpack config (#781)
Summary:
We currently configure the Babel config with environment variables: in
particular, the `SOURCECRED_BACKEND` environment variable causes Babel
to target Node instead of the browser. The relevant lines are copied
from `scripts/backend.js`.

The environment variable mechanism is slightly clunky, especially as it
requires the Webpack config module to be impure, but it works okay for
our purposes. We could adopt a more principled solution—setting the
`options` argument to the Babel loader in the backend Webpack config—but
this would require redesigning the Babel config system, which would take
a moderate amount of effort.

Test Plan:
As of this commit, `yarn backend` has bitwise identical output to
directly invoking Webpack:

```shell
$ yarn backend >/dev/null 2>/dev/null
$ shasum -a 256 bin/* | shasum -a 256
c4f7494c3ba70e5488ff4a8b44550e478a2a8b27fa96f286123f9566fd28f7be  -
$ NODE_ENV=development node ./node_modules/.bin/webpack \
> --config ./config/webpack.config.backend.js >/dev/null 2>/dev/null
$ shasum -a 256 bin/* | shasum -a 256
c4f7494c3ba70e5488ff4a8b44550e478a2a8b27fa96f286123f9566fd28f7be  -
```

wchargin-branch: backend-set-babel-flags
2018-09-05 12:10:19 -07:00
William Chargin 98a039d340
backend: use Flow in Webpack config (#780)
Summary:
Over the past few commits, I have accidentally removed all Flow errors
from the Webpack config. We can now use Flow on that file to prevent any
new errors from creeping in.

Test Plan:
Running `yarn flow` suffices.

wchargin-branch: backend-flow
2018-09-05 12:03:30 -07:00
William Chargin 4e8c89abfe
backend: export a normal Webpack config object (#779)
Summary:
Previously, our `webpack.config.backend.js` file actually exported a
function that could be used to make a Webpack configuration object.
(This is not to be confused with the late `makeWebpackConfig.js`, which
actually exported a configuration object!)

In addition to being confusing nomenclature, this was a sneaky trap for
CLI users. Invoking `webpack --config config/webpack.config.backend.js`
would actually work, but do the wrong thing: Webpack _allows_ your
configuration object to be a function, but with different semantics. In
particular, the result was that Webpack would emit the build output into
your current directory instead of into `bin/`.

This commit fixes that by making `webpack.config.backend.js` export the
Webpack configuration object for the backend JavaScript applications.
The logic to change the path is now handled by the caller, by
overwriting `config.output.path`; this is exactly [the same approach
that the Webpack CLI takes when given an `--output-path`][1], so it’s
okay with me.

[1]: 368e2640e6/bin/convert-argv.js (L406-L409)

Test Plan:
Run `yarn backend` and `yarn backend --dry-run`. Note that each runs
with appropriate output (both emitted files and console logs).

wchargin-branch: backend-webpack-config-object
2018-09-05 11:58:20 -07:00
Dandelion Mané 2779770af5
Organize weights by plugin (#773)
This commit adds PluginWeightConfig, which is responsible for
adding all the weights for an individual plugin. The top-level
WeightConfig now creates multiple PluginWeightConfigs. It also takes
responsibility for hiding the FallbackPlugin.

Test plan: The PluginWeightConfig is tested (and fairly simple). The
top-level WeightConfig is not yet tested (#604), so I manually tested
that the weights in the app still function.
2018-09-05 11:57:20 -07:00
William Chargin 846363465d
backend: remove file size measurement logic (#778)
Summary:
This logic isn’t currently useful to us. If we want this functionality,
we should consider using a Webpack plugin like `size-limit`. In the
meantime, this removes functionality from `scripts/backend.js`,
continuing on the path to #765.

Recommend reviewing with `-w`.

Test Plan:
Run `yarn backend` and `yarn backend --dry-run`, noting that each works.

wchargin-branch: backend-remove-file-sizes
2018-09-05 11:41:00 -07:00
William Chargin 7e66886a70
dep: remove `eslint-loader` (#777)
Summary:
As of #775, this is no longer used.

Test Plan:
A `git grep eslint-loader` shows no results, and `yarn test --full`
passes.

wchargin-branch: remove-eslint-loader
2018-09-05 11:33:47 -07:00
William Chargin c8175b9e3c
backend: remove superfluous file existence check (#774)
Summary:
Webpack will fail (quickly) if any required entry points do not exist.
The `scripts/backend.js` script superfluously checks this, too. This
patch removes that check.

Test Plan:
In `config/paths.js`, change `src/cli/main.js` to `src/cli/wat.js`.
Then, `yarn backend`, and note that Webpack fails quickly, with an error
“entry module not found”. Note that Webpack fails equally quickly if you
change the path of the last entry point rather than the first one.

wchargin-branch: backend-remove-exists-check
2018-09-05 11:26:22 -07:00
William Chargin 3d0f295ba7
webpack: remove superfluous linting step (#775)
Summary:
We lint separately, with `yarn lint`. There’s no need to duplicate this
effort.

Test Plan:
Introduce a lint error, for instance by adding `("unused expression");`
to `src/cli/main.js` and `src/app/App.js`. Note that `yarn lint` fails
but `yarn backend` and `yarn start` and `yarn build` succeed.

wchargin-branch: webpack-no-lint
2018-09-05 11:25:59 -07:00
William Chargin 8c0bbbc732
backend: move build dir removal into Webpack (#776)
Summary:
Both the backend and the web builds want to empty the build directory
before starting. This commit makes them use the same codepath, reducing
the amount of work that `scripts/backend.js` does so that we can more
easily remove it (#765).

Test Plan:

```shell
$ touch ./bin/wat
$ yarn backend >/dev/null 2>/dev/null
$ file ./bin/wat
./bin/wat: cannot open `./bin/wat' (No such file or directory)
```

wchargin-branch: backend-empty-backend-build-directory
2018-09-05 11:25:52 -07:00
Dandelion Mané f191995c61
Fail on console errors when testing enzyme (#769)
`testUtil.configureEnzyme` now additionally asserts, after every test,
that `console.error` and `console.warn` were not called. Tests that
explicitly expect such calls can still be written by manually re-mocking
the relevant console method (and several examples already exist).

The code that explicitly specifies this for various enzyme test files
has been removed.

Test plan: `git grep "not.toHaveBeenCalled"` shows only unrelated usage.
`yarn test` passes. Adding a spurious console.warn to a passing test
causes it to fail.

Fixes #668
2018-09-05 11:12:38 -07:00
William Chargin d2727c01ba
Fix insidious quoting bug in build test script (#772)
Summary:
This patch fixes a particularly sneaky bug. Our test script contains a
literal backtick inside single quotes. This is generally not a problem,
because backticks inside single quotes do nothing. But the contents of
the single quotes are interpreted as Bash by our test runner, and at
that time the single quotes are expanded to a command substitution.
Therefore, `grep` is invoked as if writing

    grep -e "warning: running $(yarn backend)"

at the CLI. This will actually invoke `yarn backend`!

The magnificent aspect of this bug is that it both makes the test script
slower by about ten seconds _and_ completely and silently defeats the
assertion in which it’s contained. The output of `yarn backend` contains
several blank lines. Therefore, one of the literal patterns to `grep`
contains a blank line. This causes `grep` to match _every_ line in the
error file, regardless of whether it is one of the intended messages.

This patch is the 666th PR to SourceCred. In my opinion, it deserves
this dubious honor.

Test Plan:
Note that `yarn test --full` works, but fails if one of the expected
error message patterns is deleted or munged.

Confirm the behavior by prepending `echo backend >>/tmp/log &&` to the
`yarn backend` script in `package.json`, noting that the resulting log
file contains four lines before this patch and two lines after it.
(Don’t forget to delete/clear the log file before invocations.)

Confirm the behavior of `grep` by writing:

```shell
$ printf 'things went wrong!\n' >err
$ printf 'wat\n\nwot\n' >patterns
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
1
$ printf 'wat\nwot\n' >patterns  # no empty line
$ grep -vF -e "okay" -e "warn: `cat patterns`" err; echo $?
things went wrong!
0
```

wchargin-branch: fix-build-test-quoting
2018-09-05 10:53:05 -07:00
William Chargin 1816b13525
Fix expected error message in build test (#771)
Summary:
This change should have happened in #768. However, I didn’t catch it
then because `yarn test --full` passes even before this commit, despite
the expected error being clearly wrong! It turns out that a very sneaky
bug conspires with this one to result in the test passing no matter what
kinds of warnings `yarn backend` may output. This bug is fixed in #772.

Test Plan:
Observe that the error message is now correct by comparing against the
source in `config/RemoveBuildDirectoryPlugin.js`. Then, apply #772 and
note that `yarn test --full` still passes, but does not pass when #772
is applied and this change is reverted.

wchargin-branch: fix-expected-error-message
2018-09-05 10:41:53 -07:00
William Chargin 5e0833421a
Make `execDependencyGraph` a CommonJS module (#767)
Summary:
This simplifies interfaces everywhere.

See also #216, which did the opposite of this as a temporary fix due to
a Babel/Webpack interaction that no longer exists as of #766.

Test Plan:
Note that `node bin/sourcecred.js load sourcecred/example-git` still
works (after `yarn backend`). Note that `yarn test` still works. These
demonstrate that the module works from both a Webpack context and a Node
context. Note that `git grep --name-only execDependencyGraph` yields
exactly those files touched in this commit. Note that `yarn test --full`
passes.

wchargin-branch: commonjs-execDependencyGraph
2018-09-04 22:01:22 -07:00
William Chargin 9b31905ab4
babel: transform ES6 modules to CommonJS (#766)
Summary:
The approach used by `create-react-app` (the source of this code) seems
to be the following: write everything as ES6 modules (`import`/`export`)
and bundle everything with Webpack—in particular, do not transform
modules with Babel.

This works fine until you want to also access some of these modules from
raw Node. Node only supports CommonJS modules, so these “polyglot”
modules must be CommonJS-compatible. But CommonJS modules are not nicely
compatible with ES6 modules: you need to set `exports.default` instead
of `module.exports` in the provider, and dereference `.default` after
each `require`. Babel will perform these transformations for us if we
ask it to. In this patch, we do so.

Test Plan:
Note that `yarn test --full` passes and `yarn start` still works.
(Follow-up commits will exercise this functionality.)

wchargin-branch: transform-es6-modules
2018-09-04 21:47:36 -07:00
William Chargin ddc93826be
Rename `makeWebpackConfig` to `webpack.config.web` (#770)
Summary:
The distinction was useful while `makeWebpackConfig` was being developed
(between #562 and #570), but is now confusing: we have a web config and
a backend config, and it is clearer if we name them as such.

Test Plan:
All of `yarn start`, `yarn build`, and `yarn test --full` work.

wchargin-branch: webpack-config-web
2018-09-04 21:45:10 -07:00
William Chargin 86c1b2e068
webpack: empty build dir instead of removing it (#768)
Test Plan:
Run `mkdir /tmp/out; cd /tmp/out; python -m SimpleHTTPServer`. In
another shell, run `./scripts/build_static_site.sh --target /tmp/out`.
Then, `curl localhost:8000`. Before this commit, this would have yielded
an `OSError` because the cwd of the Python process had been removed.
As of this commit, it works fine.

Also, run `git grep -c rimraf` and note only `yarn.lock:15`.

wchargin-branch: webpack-empty-build-directory
2018-09-04 21:44:09 -07:00
Dandelion Mané a133a7252f
Fix a bug that can lock the cred explorer UI (#764)
Currently, it's possible to lock the cred explorer UI via the following
sequence of actions:

1. Press the analyze cred button
2. While it is running, modify the weights

After following this repro, the UI is stuck: it will say "loading"
forever, and the analyze cred button is disabled.

The issue is that loadGraph and runPagerank do not apply their success
(or failure) state transitions if they are pre-empted by another state
change. If a repo change occurs, that's the right behavior: the repo
change puts the state back to `"READY_TO_LOAD_GRAPH"`, which means the
UI is ready to re-load, and showing the PageRank results for the wrong
repo would be very confusing.

However, if an edge evaluator change occurs while loadGraph or
runPagerank is happening, the state is left in the "LOADING" status
(which means the analyze cred button  is disabled).

This commit fixes the issue via a refactor: per @wchargin's suggestion,
responsibility for the edge evaluator moves from the state module out to
`credExplorer/App.js`. This dramatically simplifies the state module, as
we no longer need a `Substate` concept: we can simplify the state into a
single sequence of states.

As of the refactor, the bug is impossible.

Test plan: Unit tests have been updated to maintain coverage on the
refactored code. I manually tested that the bug no longer repro's, and
that the cred explorer UI continues to function. I did not add a new
test to protect against regression, because in the new codepath, the bug
is basically impossible.
2018-09-04 19:41:37 -07:00
Dandelion Mané 67c443af99
Add default weights for edge types (#762)
This commit modifies the edge type so that it has a
`defaultForwardWeight` and `defaultBackwardWeight`, and these defaults
are respected by the `WeightConfig`.

I came up with reasonable-seeming defaults for the Git and GitHub
plugins; these will undoubtably be more methodically tuned in the
future.

Test plan: `yarn test` passes, also opening the cred explorer now has
the specified default weights in the WeightConfig. (Note that the
forward/backward directions are reversed as described in #749.)
2018-09-04 16:00:49 -07:00
Dandelion Mané c7e5a3b87d
Configure forward/backward edge weights separately (#749)
This commit introduces a new component, `EdgeTypeConfig`, which is
responsible for configuring the weights for a given edge type. The
config creates two `WeightSlider`s: one for the forward direction, and
one for the backward direction. The `DirectionalitySlider` is no longer
used, and is removed. This fixes #596.

So as to avoid confusion, we now describe every edge with variables, as
in 'α REFERENCES β', and clarify that the weight modifies how cred flows
from β to α. This necessitated the creation of an `EdgeWeightSlider`,
local to the `EdgeTypeConfig`, which sets up a `WeightSlider` with the
necessary greek characters.

The EdgeTypeConfig is tested, so this is continuing progress towards
solving #604.

Test plan: I manually verified that modifying edge weights has the
expected effect on cred scores. Also, some new unit tests are included.
2018-09-04 15:37:00 -07:00
Dandelion Mané 4cd45c77fc
Factor out NodeTypeConfig (#763)
This factors `NodeTypeConfig` out of the `WeightConfig` component. The
scope for a `NodeTypeConfig` is that it configures a single node type.
Right now it just renders a single `WeightSlider`, but I like factoring
out both for consistency with the `EdgeTypeConfig` (see #749) and
because I expect we may want to add more complexity later.

Test plan: The new component has some tests, also I manually tested the
frontend.
2018-09-04 12:52:53 -07:00
Dandelion Mané 44407b5520
Combine loadGraph and runPagerank into one button (#759)
* StateTransitionMachine.loadGraph reports success

Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.

Test plan: Unit tests included.

* Add StateTransitionMachine.loadGraphAndRunPagerank

This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.

Progress on #586.

Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.

* Combine loadGraph and runPagerank into one button

Resolves #586. The new button is called "Analyze cred".

Test plan: Unit tests, also I tested it manually.
2018-09-03 14:34:14 -07:00
Dandelion Mané 1135141054
Add StateTransitionMachine.loadGraphAndRunPagerank (#758)
* StateTransitionMachine.loadGraph reports success

Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.

Test plan: Unit tests included.

* Add StateTransitionMachine.loadGraphAndRunPagerank

This methods combines `loadGraph` and `runPagerank` into one method
which internally chains the two method. `runPagerank` is only called if
`loadGraph` was successful.

Progress on #586.

Test plan:
The new method has attached unit tests. I implemented the unit tests via
mocking, which seemed quite convenient as the method is basically a
wrapper for chaining two other function calls.
2018-09-03 14:33:52 -07:00
Dandelion Mané 241905f6d8
StateTransitionMachine.loadGraph reports success (#757)
Step one towards #586. This will enable us to chain runPagerank after
loadGraph only if the load went through successfully.

Test plan: Unit tests included.
2018-09-03 14:33:25 -07:00
Dandelion Mané b92d6138d3
Improve GitHub rate limit error message (#755)
Fixes #732; see that issue for context.

Test plan:
The success case still works (verified that loading
sourcecred/sourcecred works).

I haven't tested the error case, as getting a real RATE_LIMIT_EXCEEDED
form GitHub is time-consuming, and has only happened once in practice.
I'm pretty confident the code works because it's a simple adaptation of
the code that catches other cases.
2018-09-03 14:33:17 -07:00
Dandelion Mané f3fbb3940b
Remove vestigial GITHUB_DELAY_MS code (#756)
As of #699, the GITHUB_DELAY_MS code is vestigial. It should be removed.

Test plan: After rebuilding the backend, loading a repository still
works.
2018-09-03 11:38:27 -07:00
William Chargin eb8f2b975b
Make js_bundle_path test POSIX-compliant (#754)
Summary:
In #715, I used Bash arrays for convenience. Our tests should run under
POSIX `sh` (as on Travis and standard GNU/Linux). This patch
reimplements the check using only POSIX features.

Fixes #752.

Test Plan:
As is, `yarn test --full` passes on GNU/Linux and macOS(+GNU coreutils).

Change the glob from `main.*.js` to `*.js` and note that running the
test emits an error:

```
fatal: multiple main bundles found:
    build_output/output_NO_REPOS/static/js/main.6307f660.js
    build_output/output_NO_REPOS/static/js/ssr.e92af807.js
```

Change the glob from `main.*.js` to `nope.*.js` and note that running
the test emits an error:

```
fatal: no main bundle found
```

Revert the glob to normal and note that all tests run and pass.

(To run tests, `./test_build_static_site.t --chain-lint --long -v` from
the `sharness/` directory.)

wchargin-branch: posix-bundle-check
2018-09-03 10:51:16 -07:00
William Chargin d4a9e0daa4
Add ":" as a shell-safe character (#753)
Test Plan:
Running `./test_build_static_site.t --long -v` no longer detects the
feedback URL as unsafe. (Prior to this commit, it emitted a message to
this effect.) The build is still broken on Linux for other reasons, but
works on macOS or any other system where `sh` resolves to Bash.

As a regression test, the “potentially unsafe argument” warning has been
made to actually fail the test case. To verify this, remove `:` from the
list of `unusual_chars`, run the test, and note that it fails outright.

wchargin-branch: shell-safe-colon
2018-09-02 23:22:53 -07:00
Claire L 5abe16144f Add Discord link and logo to navbar (#587) (#593)
Summary:
To facilitate communication and contribution, the Discord
invitation has been linked.

Test plan:
Visual inspection and manual link clicking
2018-09-02 23:15:36 -07:00
William Chargin 0a08783424
Remove OClif entirely (#745)
Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` still works.
Note that `git grep -i oclif` returns no results.
Rejoice.

wchargin-branch: remove-oclif
2018-09-02 16:16:00 -07:00
William Chargin e71264f5cc
Replace `oclif` with `cli` (#744)
Summary:
This commit changes the CLI to use the code in `cli` instead of `oclif`.
A subsequent commit will remove the dependency on OClif altogether.
Resolves #580.

Test Plan:
Note that `yarn backend; node bin/sourcecred.js help` works. Note that
the documentation in the README is still correct.

wchargin-branch: cli-replace-oclif
2018-09-02 16:11:56 -07:00
William Chargin 17172c2d96
cli: implement `load` (#743)
Summary:
This ports the OClif version of `sourcecred load` to the sane CLI
system. The functionality is similar, but the interface has been
changed a bit (mostly simplifications):

  - The `SOURCECRED_GITHUB_TOKEN` can only be set by an environment
    variable, not by a command-line argument. This is standard practice
    because it is more secure: (a) other users on the same system can
    see the full command line arguments, but not the environment
    variables, and (b) it’s easier to accidentally leak a command line
    (e.g., in CI) than a full environment.

  - The `SOURCECRED_DIRECTORY` can only be set by an environment
    variable, not by a command-line argument. This is mostly just to
    simplify the interface, and also because we don’t really have a good
    name for the argument: we had previously used `-d`, which is
    unclear, but `--sourcecred-directory` is a bit redundant, while
    `--directory` is vague and `--sourcecred-directory` is redundant.
    This is an easy way out, but we can put the flag for this back in if
    it becomes a problem.

  - The `--max-old-space-size` argument has been removed in favor of a
    fixed value. It’s unlikely that users should need to change it.
    If we’re blowing an 8GB heap, we should try to not do that instead
    of increasing the heap.

  - Loading zero repositories, but specifying an output directory, is
    now valid. This is the right thing to do, but OClif got in our way
    in the previous implementation.

Test Plan:
Unit tests added, with full coverage; run `yarn unit`.

To try it out, run `yarn backend`, then `node bin/cli.js load --help` to
get started.

I also manually tested that the following invocations work (i.e., they
complete successfully, and `yarn start` shows good data):

  - `load sourcecred/sourcecred`
  - `load sourcecred/example-git{,hub} --output sourcecred/examples`

These work even when invoked from a different directory.

wchargin-branch: cli-load
2018-09-02 16:07:46 -07:00
William Chargin d685ebbdd4
cli: add a `common` module for environment vars (#742)
Summary:
This includes environment variables to specify the SourceCred directory
and the GitHub token. Parts of this may change once #638 is resolved.

Test Plan:
Unit tests included, with full coverage; run `yarn unit`.

wchargin-branch: cli-common
2018-09-02 16:03:38 -07:00
William Chargin ff2d4f2fd8
cli: add `main`, `sourcecred`, and `help` (#741)
Summary:
This commit includes a minimal usage of an actual CLI application. It
provides the `help` command and no actual functionality.

Test Plan:
Unit tests added, with full coverage. To see it in action, first run
`yarn backend`, then run `node bin/cli.js help`.

wchargin-branch: cli-beginnings
2018-09-02 15:53:24 -07:00
William Chargin 4c433d417e
cli: add command infrastructure and test utils (#740)
Summary:
This commit introduces the notion of a `Command`, which is simply a
function that takes command-line arguments and interacts with the real
world. This infrastructure will enable us to write a well-tested CLI.

The `Command` interface is asynchronous because commands like `load`
need to block on promise resolution (for loading GitHub and Git data).
This is annoying for testing, but does not actually appear to be a
problem in practice.

Test Plan:
Unit tests added. See later commits for real-world usage.

wchargin-branch: cli-command-infrastructure
2018-09-02 15:48:47 -07:00
William Chargin 1f4a6395c8
cli: rename existing system from `cli` to `oclif` (#739)
Summary:
Per #580, we aim to remove OClif. To do so, we move the old system to a
directory `oclif`, and will create the new system in the now-vacant
`cli` directory.

Test Plan:
Note that `yarn backend` still builds, that `node bin/sourcecred.js`
still has `help` and `load`, and that `git grep -wc cli` yields only
`yarn.lock:9`.

wchargin-branch: rename-cli-to-oclif
2018-09-02 15:44:30 -07:00
Dandelion Mané 931f07de13
Compress the RelationalView by removing post body (#747)
Our serialized RelationalView can get quite large - in the case of
TensorFlow it's over 190MB. This is a problem, as GitHub pages have a
hard cap of 100MB on hosted files.

As a temporary workaround, this commit introduces a method,
`compressByRemovingBody`, which strips away the bodies of every post. In
the longer term, we'll need a solution that scales with larger
repositories, e.g. sharding the relational view into smaller pieces.

Test plan: Unit tests were added. I've manually confirmed that the
newly-generated views are smaller (2.1MB vs 3.3MB), and that the
frontend continues to function.
2018-09-02 00:16:09 -07:00
William Chargin 7f81337d74
Store GitHub data gzipped at rest (#751)
Summary:
We store the relational view in `view.json.gz` instead of `view.json`,
taking advantage of the isomorphic `pako` library for gzip encoding and
decoding.

Sample space savings (note that post bodies are included; i.e., #747 has
not been applied):

       SAVE     OLD (B)     NEW (B) REPO
      89.7%       25326        2617 sourcecred/example-github
      82.9%     3257576      555948 sourcecred/sourcecred
      85.2%    11287621     1665884 ipfs/js-ipfs
      88.0%    20953425     2520358 gitcoinco/web
      84.4%    38196825     5951459 ipfs/go-ipfs
      84.9%   205770642    31101452 tensorflow/tensorflow

<details>
<summary>Script to generate space savings output</summary>

```shell
savings() {
    printf '% 7s % 11s % 11s %s\n' 'SAVE' 'OLD (B)' 'NEW (B)' 'REPO'
    for repo; do
        file="${SOURCECRED_DIRECTORY}/data/${repo}/github/view.json.gz"
        if ! [ -f "${file}" ]; then
            printf >&2 'warn: no such file %s\n' "${file}"
            continue
        fi
        script="$(sed -e 's/^ *//' <<EOF
            repo = '${repo}'
            pre_size = $(<"${file}" gzip -dc | wc -c)
            post_size = $(<"${file}" wc -c)
            percentage = '%0.1f%%' % (100 * (1 - post_size / pre_size))
            p = '% 7s % 11d % 11d %s' % (percentage, pre_size, post_size, repo)
            print(p)
EOF
        )"
        python3 -c "${script}"
    done
}
```

</details>

Closes #750.

Test Plan:
Comparing the raw old version with the decompressed new version shows
that they are identical:

```
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json \
> shasum -a 256 -
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f  -
$ <~/tmp/sourcecred/data/sourcecred/example-github/github/view.json.gz \
> gzip -dc | shasum -a 256
63853b9d3f918274aafacf5198787e18185a61b9c95faf640a1e61f5d11fa19f  -
```

Additionally, `yarn test --full` passes, and `yarn start` still loads
data and runs PageRank properly.

wchargin-branch: gzip-relational-view
2018-09-01 10:42:30 -07:00
William Chargin f1a6b37524
Allow backend `process.env` to see the runtime env (#748)
Summary:
This is a follow-up to #746, wherein we exposed our fixed `env` to the
backend applications. We now extend that environment so that it can also
access the user’s runtime environment—i.e., the native values of
`process.env`.

(This is in contrast to the frontend bundles `main.js` and especially
`ssr.js`, where this is not and should not be the case: the environment
must be fixed at build time.)

Test Plan:
Add to the top of `async run()` in `src/cli/commands/load.js`:

```js
    console.log(require("../../app/version").VERSION_SHORT);
    console.log(process.env.AT_RUNTIME);
```

Run `yarn backend` and `AT_RUNTIME=wat node bin/sourcecred.js load`.
Ensure that the version number and the string `wat` are both printed.
(Before this patch, the string `undefined` would be printed instead of
`wat`.)

wchargin-branch: backend-extensible-env
2018-08-31 16:34:18 -07:00
Dandelion Mané 8009e20e5b
Fix crash on repos with underscores and dots (#738)
The GitHub regex in urlIdParse.js incorrectly disallowed repo names with
underscores and dots. Fixes #721.

To mitigate errors like this in the future, code which uses regexes to
find owners and repos has been modified to all depend on the same regex
pattern.

Test plan:
Unit tests have been updated to include the failure case (they correctly
failed), and then code was updated so that the tests pass again.

Also, I manually verified that loading ipfs/js.ipfs.io no longer fails.

Paired with @wchargin
2018-08-31 16:18:47 -07:00