Document BS2 deprication and BS5 migration. (#237) #242
Conversation
melaniekung
added
admin manual
melaniekung
force-pushed
the
dev/issue-237-BS5-theme-migration
branch
from
0b80524 to
3b53dd1
Compare
melaniekung
changed the title
melaniekung
force-pushed
the
dev/issue-237-BS5-theme-migration
branch
from
3b53dd1 to
073c861
Compare
melaniekung
changed the title
melaniekung
force-pushed
the
dev/issue-237-BS5-theme-migration
branch
2 times, most recently
from
09ee5fb to
a594eea
Compare
| git and publish them in a remote repository hosted by GitHub so we can enable | ||
| others to contribute in the development. The repository is open source so you | ||
| can use it for your own reference, see https://github.com/artefactual-labs/atom-theme-corcovado. | ||
| The following instructions assumes that you already have AtoM installed in your |
There was a problem hiding this comment.
2 quick comments here - first, I think it should be "The following instructions assume" (i.e. singular assume, because the instructions are plural - he assumes; they assume).
More importantly, I am not sure if the diff is showing me this correctly, but: it seems like this sentence just cuts off? If anything, I think you can probably remove that the whole sentence starting with "The following...", as it's no longer really needed with the way that things are structured.
If you still wanted to include some sort of reference to the dev environments, and a link to the docs for them, I would suggest adding a tip admonition right under the section title below, something like:
.. TIP::
The following instructions assume you already have an AtoM development environment set up
locally. If not, we have two development-friendly environments for AtoM - see:
* :ref:`dev-env-vagrant`
* :ref:`dev-env-compose`
a594eea to
b129108
Compare
melaniekung
force-pushed
the
dev/issue-237-BS5-theme-migration
branch
from
b129108 to
65612cf
Compare
| <https://nodejs.org/en/download>`_ and export the PATH variable. | ||
|
|
||
| The tarball is missing two required files for this: copy the | ||
| `package.json <https://github.com/artefactual/atom/blob/stable/2.7.x/package.json>`_ |
There was a problem hiding this comment.
Sorry a bit late to the party @melaniekung and @fiver-watson. We should add package-lock. json to this list of files.
There was a problem hiding this comment.
hi @jraddaoui, we've discussed internally and decided not to include the package-lock.json to this list since the tarball includes build versions, meant to be ready for use and not for development env. however, if you have other reasons for including the package-lock.json here, please let us know.
There was a problem hiding this comment.
We should, these are instructions to re-build pieces of that version and the lock file is important for that process (more than for development). Even if we only allow small version variations in package.json, we want to be "sure" the re-build uses the same dependency tree that was tested. There are other reasons too:
https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json
| <https://nodejs.org/en/download>`_ and export the PATH variable. | ||
|
|
||
| The tarball is missing two required files for this: copy the | ||
| `package.json <https://github.com/artefactual/atom/blob/stable/2.7.x/package.json>`_ |
There was a problem hiding this comment.
Hmm, good call.
While making changes, I just realized that this link is to our stable/2.7.x repository - but if we don't want to have to update this GitHub link with every release, I would suggest that you update that link and the one in the other file to:
`package.json <https://github.com/artefactual/atom/blob/HEAD/package.json>`_
Using HEAD will ensure that the link always points to the latest commit on the default branch, kind of like using /latest/ in our documentation web links instead of a version number!
There was a problem hiding this comment.
We don't want them to use the qa/2.x version of those file in the stable release. Maybe this could be solved using the version var (from this docs) in the URL so it's updated based on that?
…ation Document BS2 deprication and BS5 migration. (#237)
This commit should address all documentation changes related to themeing.