Summary:
Previously, we used `backend` to build the backend and `build` to build
the frontend, which was a bit confusing. Now, `build:backend` and
`build:frontend` build the respective components, and `build` builds
both in parallel.
Test Plan:
Run `rm -r bin build` and then `yarn build`, and note that the `bin` and
`build` directories are recreated.
wchargin-branch: build-unified-cmd
This commit removes the `core/project.js` type. It was a sort of
"kitchen sink" config file where we just threw any random thing. This
was not a good approach to config, so now we let each plugin provide
it's own config format, and we no longer need (or want) the project
type.
I removed a test from the compatIO module, although we'll probably
remove that too ere long.
Test plan: `yarn test`.
This adds a simple sharness snapshot test for the new instance system,
in the model of the old [test_load_example_github.t][old].
I've setup a very simple test instance with the GitHub and Discourse
plugins, and we verify that the output generated by running `load`,
`graph`, and `score` in succession is stable. (Cache is not
persisted.) This is a nice sanity check to verify that nothing ever gets
totally broken; we'll still want to add unit testing for more specific
features and edge cases.
Test plan:
`yarn test --full` passes.
If you `rm -rf sharness/test-instance/output`, then `yarn test --full`
fails.
If you then run `./scripts/update_snapshots.sh`, then the output
directory will be restored; afterwards `yarn test --full` passes again.
To verify that the snapshots are valid, you can test them with the
frontend:
`yarn start --instance sharness/__snapshots__/test-instance`
If you are actually debugging this script, rather than using
`yarn test --full` you'll want to invoke
`(cd sharness; ./load_test_instance.t -l -v)`
[old]: https://github.com/sourcecred/sourcecred/blob/v0.6.0/sharness/test_load_example_github.t
This commit removes old logic related to loading projects in the context
of "data directories". In particular, almost all of the `src/backend`
directory is removed (and I suspect the remaining files should come out
too; definitely the compat_io module is superseded by the new Parser
based approach). The old style loaders are also removed, as they are
rendered redundant by the new CliPlugin interface.
Test plan: `yarn flow` passes, and none of the load-bearing (ha ha)
parts of the current setup are impacted, i.e. there are no changes to
the CLI. Thus, we may be confident that everything is still working.
This commit removes (both) legacy explorer UIs, since they have been
replaced as of #1894.
Test plan: No code that is currently in use has been touched, `yarn test`
passes. Clean deletes are easy.
This commit integrates a new version of the cred explorer, which is
compatible with the new instance-based cred. It combines some of the
benefits of the v1 explorer (shows cred source decomposition) and the v2
explorer (shows cred-over-time for users). It's still very basic, and it
lacks an important feature of both v1 and v2 in that it doesn't allow
recomputing cred scores with new weights.
Nonetheless, it's a big improvement over having no functional ui at all!
Test plan: Manually test with a real instance, e.g. via
`yarn start --instance $PATH`
This adds `inflows` and `outflows` methods to the CredView.
The CredView module is known-temporary because it is built on top of
TimelineCred, which is going to be replaced by CredRank. Also, it's
extremely hard to write proper tests for, because TimelineCred is rotten
and doesn't have clearly defined semantics on small, easily tested
graphs.
Therefore, I haven't written unit tests for this; I have already used it
in downstream UIs and it does show consistent and intuitive results, so
I am reasonably confident that it works.
I expect this will stay in the codebase for 1-3 months before being
replaced by new (and thoroughly tested) views built over CredRank's
output data.
Test plan: Downstream usage in commits, e.g. see UI in:
9b9ec4bf1d
Summary:
By making the `Compatible<T>` type definition transparent, it becomes
valid to pass a compat value to the `parse` or `parseOrThrow` method of
a `Combo.Parser`. Ideally, we’d be able to more precisely specify the
relationship (`instance Json t => Json (Compatible t)`), or failing that
at least uniformly require that `T: JsonObject` and keep the type
definition opaque, but the former isn’t possible in Flow and the latter
is [blocked on a Flow bug][1], so this will have to do for now.
Fixes#1875.
[1]: https://github.com/facebook/flow/issues/8405
Test Plan:
Running `git grep 1875` no longer turns up anything relevant.
wchargin-branch: compat-expose-jsonobject
Summary:
This is more general: a mutable type is a subtype of its corresponding
read-only type. Using the covariant form lets structures like `string[]`
be subtypes of `JsonObject`, as `T[]` is not (in general) a subtype of
`(T | U)[]` but is a subtype of `$ReadOnlyArray<T | U>`.
Test Plan:
Demonstration type-level tests added.
wchargin-branch: jsonobject-readonly
We need the `--version` flag, both for general goodness, and because our
Docker publish pipeline depends on it. While I was at it, I
re-integrated the `help` command as well, along with some unit testing.
Test plan: `yarn test` passes; run `yarn backend` and then run the
command both without args (to see a dummy help message), or with the
`--help` or `help` args, or with `--version` args. Unit tests cover all
thsi behavior.
This moves all of the `cli2` command files into the `cli` directory,
since we are making the instance CLI the canonical CLI.
Test plan:
`yarn test --full` passes; `git grep cli2` returns no hits.
This commit removes the old CLI, which is being replaced by the new
instance-system based approach (currently called cli2).
This also removes sharness tests on the old CLI, as well as the
associated snapshots. This is a little unfortunate since the GitHub
snapshot did provide some validation against changes to the plugin; we'd
do well to re-integrate such a system when adding testing to cli2.
Test plan: `yarn test --full` passes. Since this is just a removal,
there's not much that can go wrong.
Summary:
One weird hack needs to be slightly amended; everything else still works
given recent changes to suppression comments.
Test Plan:
Running `yarn flow` passes as is, but fails if a suppression comment is
removed.
wchargin-branch: flow-v0.127.0
Summary:
The latest versions of Flow restrict suppression comments to the fixed
names `$FlowExpectedError` or `$FlowFixMe`. This patch replaces uses of
our custom suppression comment with `$FlowExpectedError`, as we don’t
use suppressions to mean “fix me; need to add types later” but instead
as expected type errors in tests (almost exclusively).
Generated by changing the suppression in `.flowconfig` and then running:
```
git ls-files -z '*.js' |
xargs -0 sed -i -e 's/\$ExpectFlowError/$FlowExpectedError/g'
```
To check the changes in non-test files, run
```
git show ':!*.test.js'
```
and note that all changes are in fact expected errors rather than TODOs.
Test Plan:
Check that `yarn flow` passes as is, but fails if a `$FlowExpectedError`
comment is deleted.
wchargin-branch: flow-standard-suppressions
Summary:
This removes a SourceCred-specific hack because the upstream issue has
been fixed. Removing the hack is important because it uses a suppression
comment and thus hampers upgrading to latest Flow.
The libdef portion of this change was generated by removing the old file
and then running `flow-typed install express@4.17.1`.
Test Plan:
That `yarn flow` passes suffices.
wchargin-branch: libdefs-express
Now that we're making the new UI the canonical one, we can remove
references to calling it `build2` or `start2`.
Test plan: `yarn start --instance $PATH` and `yarn build` both work.
`yarn test --full` passes.
This commit removes the old homepage entirely. This is a prelude
to removing the react-router dependency, which is needed to unblock work
on the initiatives editor.
Because the homepage is gone, there's now no frontend included with
SourceCred. As such, we should merge #1873 alongside this one, so that
our README doesn't give any patently false information to our users.
Test plan: `yarn test` passes. `yarn start` and `yarn build` are no
longer commands. (`yarn start2` and `yarn build2` will be renamed
later).
This adds a proper parse for core.Weights, and modifies the `fromJSON`
method to use that parser.
This means we now have runtime validation that all Weights files are
actually valid.
Note: I made a tweak to the eslint config so as to allow `_`s in names
when followed by a sequence of numbers, so that types like `Data_0_1_0`
(for including version strings) do not trigger lint failures.
Test plan: All existing tests pass, and I added a new test to validate
that we keep supporting the 0.2.0 format going forward.
Our README.md has a hodge-podge of information about using SourceCred.
Much of it is about to go entirely out of date because we are killing
the old CLI and frontend.
As such, I'm cleaning out and greatly simplifying the README, so we
won't give our users inaccurate information. I didn't make an effort at
replacing the documentation, that will come afterwards once we're ready
to document the instance system.
Creates a class to handle the implementation
of fetching data from the Discord server and
saving it to the local mirror.
Test plan:
Unit tests added.
paired with @beanow
This adds a fromRaw method to the Address module so that we may properly
parse serialized addresses. It has a type signature of `string =>
Address`, and throws an error if the string is not a valid address.
I basically copied the implementation out of the `assertValid` method. I
considered deduplicating them (i.e. having `assertValid` simply call
`fromRaw`), but `assertValid` had some extra logic around accepting a
prefix for the error message, and it felt simpler to simply copy+paste
rather than trying to wrap it.
Test plan:
I added unit tests; `yarn test` passes.
Summary:
An optional second parameter to `C.dict` can now be used to refine keys
to a subtype of string: e.g.,
C.dict(C.number, C.fmap(C.string, NodeAddress.fromRaw))
is a `C.Parser<{[NodeAddressT]: number}>`. Before this change, `C.dict`
could only parse objects whose key type ranged literally over `string`.
Note that the argument order is `C.dict(valueParser, keyParser)`, not
the other way around. This is because `C.dict(valueParser)` is the more
general use case (you _need_ a value parser), and it’s simpler to have
the optional argument be at the end. (Analogously, the `forEach` methods
on `Array`s and `Map`s take a callback accepting `(value, key)`.)
Test Plan:
Unit tests included, retaining full coverage. Existing tests for the
experimental Discord plugin, which uses `C.dict`, also still pass.
wchargin-branch: combo-dict-key
This adds two methods to cli2/common.js:
- loadJson, which loads a JSON file from disk and then parses it
- loadJsonWithDefault, which loads a JSON file from disk and parses it,
or returns a default value if the file is not present.
Both methods are well documented and well tested.
Test plan: `yarn test`; see included unit tests which are thorough.
This adds Combo parsing support to the compatible module. Now, rather
than writing `fromJSON` methods which implicitly take any, we can
instead write typesafe `parseJSON` methods which will parse compatible
headers, and then choose a version-appropriate parser.
Test plan: Added unit tests; `yarn test` passes.
Just a more sensible name since it's not a homepage of any sort.
Test plan: Flow passes, `yarn build2` and `yarn start2` both succeed.
Also: `git grep homepage2` gets no hits.
Now you pass `--instance PATH` rather than `--instance=PATH` which is
agreed to be much much better. (It tab completes.)
Test plan: Do the obvious thing. :)
Per discussion with @hammadj, @topocount, and @wchargin, we are planning
to have the frontend2 system use react-admin at the top level. Per
investigation by @topocount, react-admin conflicts with the older
version of react-router that we use.
As such, this commit wildly simplifies the homepage2 system so we no
longer have any routing, and instead we just statically render the
index.html file. We also removed the `Assets` type, not because we are
sure we don't need it, but because we didn't want to debug it while we
were all pairing. @wchargin offered to fix it up later.
Test plan:
- run `yarn start2 --instance=PATH` and observe that the "Under
Construction" message displays, along with console messages showing that
data loaded successfully.
- run `yarn build2` and copy files from `build2` into the root of a cli2
instance. Run an http server in that instance, and observe that the
frontend displays properly per instructions above.
Paired with: @wchargin
Paired with: @hammadj
Paired with: @topocount
This modifies the frontend2 so that we can load real data from cli2
instances, including:
- the root `sourcecred.json` config
- files inside the `config/` directory
- files inside the `output/` directory
This works both for the dev server and the compiled output.
In the case of the dev server, it's now necessary to provide the path to
the cli2 instance you're developing against. You can pass this via the
`--instance` argument, as in `yarn start2 --instance=/path/`, or via
the `$SOURCECRED_DEV_INSTANCE` environment variable (recommended). If
neither is provided, or if that path doesn't look like a valid instance,
an error is thrown.
In the case of the built output, given a valid sc2 instance, you can set
it up via the following:
```
cp -r build2/favicon.png build2/index.html build2/static $INSTANCE
```
Then spin up a simple http server in the $INSTANCE.
You can look at console messages in the frontend to verify that the
instance is working (note it expects to see a discourse config file, not
a GitHub config file as was used in some previous examples).
Test plan:
Setup an example Discourse instance, and then turn on the server via
`yarn start2 --instance=PATH`,
`SOURCECRED_DEV_INSTANCE=path yarn start2`,
and by manually copying in the built outputs using the instructions
above.
In each case, you can load the homepage and check the console to see
that assets loaded successfully.
The frontend build system has a bunch of logic for loading in the list
of projectIds and including them in the build. Fortunately, in
frontend2, we won't need this. This commit simply removes all that
logic.
Test plan: Grepping for `projectIds` finds no hits inside the frontend2
system, except for a comment noting that we'll be able to remove an
argument once the transition is complete. `yarn start2` still works as
expected.
This drastically streamlines the new frontend entry directory.
All the logos, the nav bar, the pages, etc, are all gone.
Now there's just a landing page that reads "Under Construction".
Test plan: Run `yarn start2`, observe that there's just an empty landing
page that we'll rebuild from.
The cli2 ("instance") system has a foundationally different assumption
about how the frontend works: rather than having a unified frontend that
abstracts over many separate SourceCred projects, we'll have a single
frontend entry per instance. This means we no longer need (for example)
to make project IDs available at build time.
Our frontend setup and server side rendering is pretty complex, so
rather than rebuild it from scratch, I'm going to fork it into an
independent copy and then change it to suit our needs.
To start here, I've duplicated the `src/homepage` directory into
`src/homepage2`, duplicated the webpack config to
`config/webpack.config.web2.js`, and duplicated the paths and
package.json scripts.
Test plan:
Run `yarn start2` and it will start an identical frontend, using the
duplicated directory. Run `yarn build2` and it will build a viable
frontend into the `build2` directory. `build2` is gitignored.
As discussed [in Discord]:
At present, cli2 is organized around the following commands:
- init: help setting up a new SourceCred instance (not yet implemented)
- load [PLUGINS...]: load data from all active plugins (default) or
specific named plugins
- graph: regenerate graph for every plugin (will likely give this
[PLUGINS...] args in the future)
- merge: merge the generated graphs together
- score: compute scores over all the graphs
I'm going to change this to fold the merge command into the score
command. This is inspired by realizing that setting the weights.json
should get incorporated in the merge command from an implementation
perspective, but in the score command conceptually.
Thus, having merge and score as separate commands is actually an
anti-affordance, it allows the user to get into a confusing state where
the graph in their output directory is in a different state from the
scores.
(also, merge takes about 2s on sc's cred instance, so it isn't a huge
performance penalty on the flow of "recompute scores with new
parameters")
cc @wchargin
Also, I wonder whether we should persist the weightedGraph to disk at
all, seeing as it is already included in the credResult.json.
Storing the graph on its own is inefficient from a space perspective,
although it can be nice to get an eyeball sense of how big the
graph is compared to the associated cred data. For now, I'm not going to
write the weightedGraph to disk, simply because it's more convenient to
implement.
If there's a need, we can add it back later.
[in Discord]: https://discordapp.com/channels/453243919774253079/454007907663740939/721603543278157905
Test plan: manually tested it on a local instance
This modifies cli2/load so that you can now provide a list of fully
scoped plugin names (e.g. sourcecred/github) and it will load only the
mentioned plugins. If any plugins aren't available, an error is thrown.
If no plugins are listed, all of the activated plugins are loaded.
Test plan: Tested locally in success and failure cases. No unit tests
atm, which matches the rest of cli2--tbh this feels like the kind of
code where the liklihood of subtle failures or regressions is low, and
it's a pain to test, so I'm content with the status quo for now.
This adds a cliPlugin to the experimental Discord plugin, along the
lines of the cli 2 plugins for GitHub and Discourse.
Test plan: I set up a Cred instance for SourceCred including our Discord
config, and successfully loaded Cred for it. Not easy to give full
reproduction instructions, not least because it requires a private
Discord bot token.
This adds support for retrieving cred-augmented nodes and edges to the
CredView class. These methods wrap the underlying Graph methods, and
return nodes and edges that also include relevant cred information
Test plan: Unit tests included; yarn test passes.
This commit adds the CredView class, an interface for Graph-aware
queries over a CredResult. To start, I just added accessor methods for
reaching the constituent data in the CredResult.
Test plan: `yarn test`
In #1738, @wchargin updated the svg logo with a cleaner path structure.
This commit re-runs the rasterize script with the new svg, which
results in much cleaner pngs without aliasing.
Test plan: The new pngs look a lot cleaner.
Summary:
The Sharness snapshots contain large data files that are effectively
blobs, but happen to be encoded as UTF-8 JSON. Consequently, they fill
up a terminal with junk when they wriggle into a `grep` or `diff` or
similar. This patch fixes that by teaching Git to treat them as binary.
If you ever want to see a raw diff (unlikely, since these files have
many kilobytes of data with no newlines), just pass `-a`/`--text`.
This isn’t perfect—e.g., it only works with Git (not, e.g., Ripgrep)—but
it’s better than nothing.
Test Plan:
Run `git grep 6961` and note that the result now shows just three lines,
including the desired text result and two “Binary file matches” notes.
Run `git grep -a 6961` and note that the monstrosity returns.
wchargin-branch: sharness-snapshots-binary
If a project.json file defines a field that belongs to a newer version but the compat version wasnt updated, the
auto-upgrade code would silently throw away the field, changing it to the default setting of null. This fixes it
by spreading the project object after adding the new field instead of before, that way if that field was defined
in the project file it wont get overwritten while still adding the empty case if it wasnt defined.
Test Plan: Add a field belonging to ProjectV052 in a project file with compat version 0.5.0 and ensure it doesn't get
overwritten.
This updates the alias "plugin" to support the experimental Discord
plugin that we recently merged into master. Without this, we can't
resolve Discord aliases.
Test plan: Validated on SourceCred's own cred instance. No unit tests
added since both modules in question (experimental discord and alias)
are going to be replaced or re-written, and the experimental-discord
plugin has no testing at all.
Since the discord branch is being used in production by many projects (MetaGame, AraCred, RaidGuild, etc), and we also want to start using it for SourceCred, it would be a good idea to merge it into master in a separate experimental-discord plugin so the proper discord plugin can be developed in parallel while allowing the other projects to stay up to date with master.
paired with @decentralion
