Ignoring the fact that the `.js*` was already _inclusive_ of `*.json`, we no
longer want `*.json` files linted (see [0] for more information), and I'm
not even sure why we would want to cover `*.md` rather than just `*.md`
(planning for a future markdown format?).
[0]: d4becdf6be
Many of these comments are unhelpful anyway and deserve to be maintained in the
"doc docs" (yes, docs for the docs), which live at https://github.com/apollographql/docs-docs/.
This the reason for this unneeded dependency traces back to the `api-box` on
https://github.com/meteor/docs, though its dependency hasn't been necessary on
any other docs deployment, including this one, since then.
I can't find any evidence in any docs deployment that `lodash` ever needed to
be a direct dependency. It's possible that this is an artifact that once lived
in an early stage of the docs and just propagated out as a result of making new
docs from old docs repositories. The `theme-example` application should help
set better precedence for this in the future.
Rather than developing the theme on individual docs deployments, that work
should now be done on the theme repository[0] and the example doc
deployment[1] since those repositories contain the infrastructure for testing
and deploying changes across all docs properties. More information on
developing the theme can be found on those repository's `README.md`s.
[0] https://github.com/meteor/hexo-theme-meteor
[1] https://github.com/meteor/theme-example
While I'm aware of the need for the `showdown` package for rendering some
content on the Meteor Docs, I'm not aware of any Apollo repository that uses
it. I've searched through the source, and cannot find any usage.
If there is a repository that uses it directly (in its `scripts/` directory),
it should be switched to using a plugin, which can explicitly declare
`showdown` as a dependency, rather than depending on it at the root of the
repository. (This is how `hexo-typescript-api-box` does it on the
`apollo-client` docs, which has correctly removed the direct dependency.)
Luckily, these repositories now have tests so if any of these fail, we should
know pretty easily and be able to add the dependency back to just those that
need it.
Generators are used for generating additional content in Hexo. While these
are great for making a blog (for example, `hexo-generator-index` to put all
your blog posts on an index page, `hexo-generator-tag` to keep a page updated
with all your trending blog tags, and `hexo-generator-archive` if you want to
store your aging content somewhere where nobody will find it), they don't seem
to be at all necessary for our purposes, which are documentation.
Unfortunately, they are in the default skeleton of a new Hexo project and they
don't seem to have ever been removed from our non-blog docs deployments,
despite the fact that they seem unnecessary.
In an effort to drastically reduce the overhead of maintaining these docs
deployments, I'm removing these as "unnecessary" until proven otherwise.
* Export `GraphQLOptions` type directly from `apollo-server-express`.
Directly export `GraphQLOptions` from the `apollo-server-express` (and
synonymously, `graphql-server-express`) main module, rather than exporting
it only from `./expressApollo`.
By using CircleCI 2.0's new Workflow feature, we can more delicately
orchestrate how the test suite runs.
Specifically, we can have individual groups of tests report their status
independently to pull-requests, and react accordingly. For example, rather
than the linting tests being lumped into the individual tests ran on each
Node.js platform version, we can now run them on their own (and also avoid
unnecessarily running them over and over again for each Node.js version!).
Each jobs test results will be reported to the pull-request directly and, if
desired, the repository's branch merge rules can be modified to allow specific
types of test failures (like linting) to be allowed.
This need for greater test granularity became quite relevant recently when many
of the outstanding PRs on this repository were failing because of "prettier"
failures which were not directly related to the files changed by the PR but
by other changes on the `master` branch. In order to confidently review and
merge those PRs, it was necessary to look through each PRs four platform
test runs and ensure _each_ was a prettier failure and not something more
substantial. This manual step left way too much room for error and was not
a confidence builder, at all!
I've also added a new test for the documentation generation to ensure that
no changes in the PR have caused the documentation to not generate, which
would result in stale documentation since the existing documentation would
simply remain in place until the problem was eventually noticed.
As a last benefit, in my observations so far, CircleCI is running more
tests, more quickly and with greater parallelisation than our previous test
provider. Recently I've seen tests finishing in about two minutes rather than
what was sometimes 5 minutes.
With a lack of comments and a fairly strict structure, it's arguable if JSON
files were ever meant to be pretty, but the method we're using right now is
a bit futile, especially considering that tools that we use to automatically
update JSON (specifically, package.json) don't run prettier themselves.
The most problematic rule is the 80 characters line limit.
Lines in JSON can be wrapped in, at most, one place: after the colon in the
key. This means that as soon as a single npm-script declaration exceeds the
line-length rule a second time, it can't be wrapped again, resulting in a
violation of the very rule being enforced.
Clearly, I've always thought that prettying JSON is a bit silly, but the
straw that broke the camel's back here is automated package.json changes by
bots which update the repository via their automated PRs.
Perhaps in a day where the JavaScript package manifest finds a new file
extension (.js?, .yaml, .toml?), it will be able to reap the glitz and glamour
of being eloquently formatted, but until then we'll have to use long-line
wrapping in our editors.
...or shorten our npm scripts.
For reasons unbeknownst to me, the changes below were only being complained
about in the CI environment but _not_ when I ran `npm run lint` locally.
It seems partially related to the OS its ran on, so I imagine there might be
some other sub-dependency at play here. In an effort to fix this, I just
spawned a Ubuntu docker image, checked out this repository and ran the same
`npm run lint`. This produced identical results to Travis, so I ran
`npm run lint-fix`, then saved the `git diff` results and have applied this
locally.
This should allow us to re-enable `prettier` in CI, though I have plans to
separate that from the actual `npm test` runs. This should result in a better
workflow for managing PRs.
* Pin `lint-staged` to maintain Node.js 4 support.
The newer version of `lint-staged` (v7) drops support for Node.js 4, a version of Node.js which is still under Maintenance LTS support and therefore we still need to test against. Since this npm would be used in our Node.js 4 test matrix, it's imperative that we not jump to v7.
Primarily because the results that prettier is giving on Travis are not the
same as those that are exhibited locally. Namely, the result of running the
`npm run lint` command locally returns different files than on TravisCI.
This makes it almost entirely impossible to determine what is wrong with the
files. It's likely enough that it's running in post-commit, but I'd be
happy to re-introduce this commit if someone can figure out what is wrong!
For now though, it's interfering with the ability to merge PRs.
As another option, perhaps CircleCI workflows would allow us to have
separate fail badges for Lint tests rather than testing the linting in every
Node.js version of the build matrix and polluting the true success and
failure of the unarguably more important tests themselves (rather than the
formatting of the code which is being tested).
While this file is actually ignored by a `.gitignore` which lives inside the
`docs` directory (as is consistent with the various docs repositories or
sub-trees), prettier is being told to ignore files in the top-level
`.gitignore` and only supports a single `--ignore-path` file.
This will allow it to skip trying to format this `.json` file.
