Summary:
This improves the documentation for the change made in #715.
Test Plan:
Note that `./scripts/build_static_site.sh --help` and `yarn sharness`
pass.
wchargin-branch: feedback-url-help-text
Summary:
We can now set, at build time, a URL to be displayed at the top of the
prototype, encouraging users to provide feedback. If the URL is not
provided, it defaults to the appropriate topic on the SourceCred
Discourse instance.
The result looks like this:
![Screenshot of the feedback URL in the prototype][screenshot]
[screenshot]: https://user-images.githubusercontent.com/4317806/44814824-a238b380-ab92-11e8-88c8-dfbae27ca496.png
Test Plan:
Unit tests added to `yarn sharness-full` and `yarn unit`.
You can run `yarn start` to see the message with the default URL, or
`SOURCECRED_FEEDBACK_URL=http://example.com/ yarn start` to specify a
custom URL.
wchargin-branch: feedback-url
Summary:
By using Git’s magic pathspecs instead of post-processing stream
operations, we reduce the pipeline to a single operation. Git implements
its own version of `grep`, so this should be platform-independent.
Previously, we had needed the `-z` argument to `grep(1)`, which is a GNU
extension.
Fixes#594.
Test Plan:
Ensure that the script passes with no output. Then,
```shell
mkdir -p src/flow-typed/
touch src/foo.js src/flow-typed.js src/flow-typed/foo.js
git add src/foo.js src/flow-typed.js src/flow-typed/foo.js
cd scripts
./ensure-flow.js
```
and ensure that script exits with code 1, empty stdout, and stderr with:
```
../src/flow-typed.js
../src/flow-typed/foo.js
../src/foo.js
```
This verifies that the pathspec is properly excluding the root directory
`flow-typed`, but not files that just happen to have `flow-typed` in
their paths.
wchargin-branch: simplify-ensure-flow
Summary:
The current version of the build script has the safe but annoying
property that the target directory must be an existing, empty directory.
It seems reasonable and convenient to allow the build script to create
the directory with `mkdir -p`. It still fails if the directory is not
empty or is a file.
Test Plan:
Unit tests updated; run `yarn sharness-full`.
wchargin-branch: build-mkdir-p
Summary:
The `node ./bin/sourcecred.js load` command invokes plugin code by
providing an output directory into which the plugin may store data.
As of this patch, it also provides a cache directory that the plugin may
use to store data that will not be available at runtime. For instance,
the Git plugin might choose to clone the repository herein, or the
GitHub plugin may choose to store partial GraphQL query results to deal
with interruptions. The contract is that the cache directory may be
removed at any time and that the plugin should continue to operate
normally.
Test Plan:
The build script has been updated and tested. Reverting the change to
the build script causes the newly added test to fail. (Each plugin has a
cache directory, though the cache directories are empty for now.)
wchargin-branch: create-plugin-cache
Summary:
This is a minimal patch to make the deploy script depend on the
separate, well-tested build script introduced in #592. The patch
suggests a number of improvements that can be made to the deploy script
itself, but I’m deferring those as they’re not on the critical path.
These are tracked at #610.
Test Plan:
Apply the below patch. Then run `./scripts/deploy.sh -n`, and verify
that the served output is correct, including the CNAME file. Verify that
when the `DEPLOY_CNAME_URL` default value is removed, the resulting
output does _not_ have a CNAME file.
Patch, for ease of building:
```diff
diff --git a/scripts/deploy.sh b/scripts/deploy.sh
index cddcece..8925e5b 100755
--- a/scripts/deploy.sh
+++ b/scripts/deploy.sh
@@ -23,7 +23,7 @@ main() {
preview_dir=
trap cleanup EXIT
- ensure_clean_working_tree
+ #ensure_clean_working_tree
build_and_deploy
}
@@ -76,8 +76,7 @@ build_and_deploy() {
"${sourcecred_repo}/scripts/build_static_site.sh" \
--target "${static_site}" \
${DEPLOY_CNAME_URL:+--cname "${DEPLOY_CNAME_URL}"} \
- --repo ipfs/js-ipfs \
- --repo sourcecred/sourcecred \
+ --repo sourcecred/example-github \
;
sourcecred_site="$(mktemp -d --suffix ".sourcecred-site")"
```
wchargin-branch: deploy-via-build
Summary:
Currently, we create the static site and deploy it all at once in
`scripts/deploy.sh`. This commit creates a new script that only builds
the static site. This has the advantage that it is easier/less scary to
change that script (because it can be tested without worrying about
deploying to a local test target), and that we can write automated tests
for it.
Test Plan:
Run `yarn sharness`; note that it completes very quickly. Then, in a
shell with your GitHub token exported, run `yarn sharness-full`. Expect
all tests to pass.
For a sanity check, you can run:
```shell
outdir="$(mktemp -d --suffix .sourcecred-site)"
./scripts/build_static_site.sh --target "${outdir}" \
--cname sourcecred.io \
--repo sourcecred/example-git \
--repo sourcecred/example-github \
;
(cd "${outdir}" && python -m SimpleHTTPServer)
```
and ensure that <http://localhost:8000/> is as expected.
One test case that is not covered is the following: _if_ the actual app
somehow tries to emit a `CNAME` file at root, _and_ our script’s logic
to catch this is broken, then we will not catch this failure. I’ve
tested the logic manually by adding `>"${cname_file}"` after definition
of that variable, but I don’t see a good way to test it automatically,
without adding flags like `--but-actually-emit-cname-too` to the build.
The compound probability of this happening is sufficiently low that this
doesn’t bother me.
wchargin-branch: build-static-site-script
Summary:
This way, we can document what STOPSHIPs are without having to tip-toe
around the phrase.
Test Plan:
Add `STOPSHIP` to `README.md`, and note that `yarn test` fails before
this change but passes after it.
wchargin-branch: stopship-markdown-okay
Summary:
Running `yarn test` (equiv. `npm test` or `npm run test`) now runs all
checks. It takes the place of the former `yarn travis`. This is more in
line with the expectation of a top-level `test` command: if it passes,
your code is good.
The `unit` command now runs Jest once, not in watch mode. It takes the
place of the former `ci-test`. To run tests in watch mode, run any of
the following:
- `yarn unit --watch`, or
- `npm run unit -- --watch`, or
- `npm unit -- --watch`.
This behavior is more consistent with the standard behavior of commands
like `make test`. It is also empirically what @wchargin and
@decentralion want most of the time.
Test Plan:
Verify that each of the scripts `test`, `unit`, and `coverage` passes.
Verify that each of the aforementioned `--watch` invocations works.
Verify that `.travis.yml` has the correct `script:` command.
wchargin-branch: reorganize-test-command
Currently, any non-yes keystroke causes the deply script to abort.
This is frustrating, as it may take the user several minutes of waiting
to get to the prompt, only to have the program inadvertently terminate.
As of this change, the deploys script patiently re-prompts until it gets
a valid input.
Paired with @wchargin.
Test plan: Ran locally and tested the yes, no, and invalid response
cases. Also ran the script through shellcheck.
The CNAME file is needed so that our custom domain will continue working
after deploys.
Test plan:
- Verified that the generated build now includes the cname file.
- Verified that if a CNAME file is already present, the script will
fail.
Paired with @wchargin
Showing our example-github and example-git repos on sourcecred.github.io
is not particularly interesting. Let's show ipfs/js-ipfs instead!
Since sourcecred/sourcecred is the last repo to load, as of #531 it will
be the default option.
Test plan: Dry run of deploy script
Summary:
For now, this contains the logic to register an `unhandledRejection`
error. I’ve removed all instances of those handlers, and `require`d this
module at every top-level entry point. (The individual CLI commands had
the handler before, but didn’t need it; conversely, the top-level CLI
entry point did not have the handler, but should have.)
Test Plan:
To test that the CLI commands still error on unhandled rejections, apply
the following patch:
```diff
diff --git a/src/v1/cli/commands/combine.js b/src/v1/cli/commands/combine.js
index b60f91e..d55b965 100644
--- a/src/v1/cli/commands/combine.js
+++ b/src/v1/cli/commands/combine.js
@@ -24,6 +24,7 @@ export default class CombineCommand extends Command {
" where each GRAPH is a JSON file generated by plugin-graph";
async run() {
+ Promise.reject("wat");
const {argv} = this.parse(CombineCommand);
combine(argv);
}
```
Then run `yarn backend` and `node bin/sourcecred.js`, and note that the
rejection handler is triggered.
wchargin-branch: unify-entry
Summary:
Previously, our CI script would run `yarn backend`, which has the
side-effect of erasing the `bin/` directory. By itself, this is not
great, but not awful. However, this frequently triggers a race condition
in Prettier, causing the `check-pretty` step of the build to fail. (More
details: https://github.com/prettier/prettier/issues/4468.)
This patch changes the CI script to build the backend scripts into a
temporary directory.
Test Plan:
Before applying this patch: `yarn backend` and then `yarn travis`. If
this consistently causes a Travis failure due to `check-pretty`, then
your machine can reproduce the race condition that we‛re trying to
eliminate. (Otherwise, you can try creating a bunch more Git history…
I’m not really sure what to say. It is a race condition, after all.)
Then, apply this patch, and repeat the above steps; note that the error
no longer occurs, and that the build output is to a temporary directory.
wchargin-branch: ci-preserve-bin
Summary:
The use of `tee /dev/stderr` failed when running as a child process
under Node for some reason. (I haven’t been able to figure out why—it
works fine when run as a standalone script or when run as a child
process under Python.) This is also technically Linux-specific, so I’ve
changed it to use a process substitution. After looking around for a
bit, there doesn’t seem to be a way to do this in a way that is
portable, uses only POSIX shell features, and doesn’t create temporary
files all at the same time, so the script is now run under `bash`.
Test Plan:
Run `yarn travis` and note that the `ensure-flow.sh` output no longer
contains the line `tee: /dev/stderr: No such device or address`.
wchargin-branch: no-tee-devstderr
Summary:
Placing `STOPSHIP` or `stopship` (or any case variant) in any file
tracked by Git will now cause a `yarn travis` failure. If you need to
use this string, you can concatenate it as `"stop" + "ship"` or
equivalent.
Test Plan:
In `travis.js`, change `"check-stop" + "ships"` to `"check-stopships"`,
and note that this causes the build to fail with a nice message. Note
that this also causes `check-stopships.sh` to fail even when invoked
from an unrelated directory, like `src`.
wchargin-branch: check-stopships
Summary:
This fixes two problems in the previous version:
- A new JS file not checked into git, but with a `@flow` directive,
would cause `ensure-flow` to fail, because one list of files was
from `git grep` and the other was from `find`.
- Only the hard-coded directories `src config scripts` were searched.
Now, we search all JS files checked into Git, except for some hard-coded
exceptions, namely `flow-typed`.
Test Plan:
1. Add `foo.js`, not checked into Git. Note that `ensure-flow` passes.
2. Add `@flow` to `foo.js`, and note that `ensure-flow` still passes.
3. Remove `@flow` from `.eslintrc.js`, and note that `ensure-flow`
fails and nicely prints the filename. (Note: this file is at the
repository root.)
4. Create a file `echo stuff >$'naughty\nfilename.js'`, and note that
`ensure-flow` has the correct behavior in both positive and
negative cases.
wchargin-branch: ensure-flow-improvements
Summary:
This way, our frontend can talk to a backend that can read from the
filesystem (among other things).
Paired with @decentralion.
Test Plan:
```
$ yarn backend
$ SOURCECRED_DIRECTORY=/tmp/srccrd yarn start
$ # verify that the browser looks good
$ mkdir /tmp/srccrd
$ echo hello >/tmp/srccrd/world
$ curl localhost:3000/api/v1/data/world
hello
$ curl localhost:4000/api/v1/data/world
hello
```
wchargin-branch: webpack-proxy
Summary:
- The value of `process.stdout.isTTY` is either `true` or `undefined`.
Flow (reasonably) dislikes this, so we add an explicit check.
- More `package.json` burnination.
Test Plan:
Note that `require("./package.json").proxy === undefined` in the Node
console, and that `yarn start` works.
wchargin-branch: flow--scripts-start
- scripts/backend.js: We incorrectly set an environment variable to
a boolean, when in fact it must be a string. Fixed it to set a string
value "true", and updated usage in config/babel.js
- scripts/test.js: No changes
- scripts/build.js: Removed a call to printHostingInstructions, so that
we don't need to require the package.json.
Test plan:
`yarn travis --full` passes, and the SourceCred cli still works.
This script ensures that either //@flow or //@no-flow is present in
every js file. Every existing js file that would fail this check has
been given //@no-flow, we should work to remove all of these in the
future.
Test plan:
I verified that `yarn travis` fails before fixing the other js files,
and passes afterwards.
Summary:
Consequently, Babel won’t transform classes to their roughly equivalent
ES5 counterparts, etc.
Test Plan:
Create `src/classy.js` with `class X {}; console.log(X);`. Then, add a
build target for `classy: resolveApp("src/classy.js"),` in `paths.js`.
Use `yarn backend` and inspect the contents of `bin/classy.js`; in
particular, look at the definition of `X` (whatever the argument to
`console.log` is). Before this commit, the result will be a big
complicated mess. After this commit, it will be `class X {}`.
Note also that `yarn travis --full` passes, indicating that the two
manual tests, which call out to the utilities in `bin/`, still work.
wchargin-branch: target-node
Summary:
Running `yarn backend` will now bundle backend applications. They’ll be
placed into the new `bin/` directory. This enables us to use ES6 modules
with the standard syntax, Flow types, and all the other goodies that
we’ve come to expect. A backend build takes about 2.5s on my laptop.
Created by forking the prod configuration to a backend configuration and
trimming it down appropriately.
To test out the new changes, this commit changes `fetchGitHubRepo` and
its driver to use the ES6 module system and Flow types, both of which
are properly resolved.
Test Plan:
Run `yarn backend`. Then, you can directly run an entry point via
```
$ node bin/fetchAndPrintGitHubRepo.js sourcecred example-repo "${TOKEN}"
```
or invoke the standard test driver via
```shell
$ GITHUB_TOKEN="${TOKEN}" src/backend/fetchGitHubRepoTest.sh
```
where `${TOKEN}` is your GitHub authentication token.
wchargin-branch: webpack-backend
Reorganize the code so that we have a single package.json file, which is at the root.
All source code now lives under `src`, separated into `src/backend` and `src/explorer`.
Test plan:
- run `yarn start` - it works
- run `yarn test` - it finds the tests (all in src/explorer) and they pass
- run `yarn flow` - it works. (tested with an error, that works too)
- run `yarn prettify` - it finds all the js files and writes to them
William Chargin
Dandelion Mané
Dandelion Mané
