-
Notifications
You must be signed in to change notification settings - Fork 7
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Record decisions for update to the repository organisation ahead of v5
- Loading branch information
1 parent
e2c03ae
commit 9c85982
Showing
1 changed file
with
122 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
# Decision Record Title | ||
|
||
Update our repository structure to prepare v5 | ||
<!-- The title should be a short present tense imperative phrase, less than 50 | ||
characters, like a git commit message. --> | ||
|
||
## Decision | ||
|
||
<!-- An overview of the decision made. --> | ||
|
||
To prepare our work for v5, we've agreed to make the following changes to the files in our repository and their location: | ||
|
||
1. Move the sources (currently `src` ) and built package files (currently `package`) under the same npm package. We'll keep shipping only the built files to npm for now, excluding the source files from the package (either via `.npmignore` or using the `files` field of `package.json`) | ||
2. Remove the built package files (currently `package`) from the version control | ||
3. Update our release process to make sure we keep checking the changes (possibly manually) between the package to be released and the previous version | ||
4. Leave `dist` in its current location for now (we may revisit whether it's in version control or at the root in the future) | ||
|
||
This will yield the following structure for our repository: | ||
|
||
``` | ||
govuk-frontend // The root of our repository | ||
├── ... // Irrelevant files of folder omitted for brevity | ||
├── dist // Not touching the location, naming might change for clarity | ||
├── package // Naming might change for clarity | ||
│ ├── src // Sources are now here, in version control, not shipped to `npm` | ||
│ │ └── govuk // Structure inside `src` is untouched | ||
│ ├── dist // Built files are now here, not in version control, ships to npm | ||
│ │ └── govuk // Keeps creating a namespace for Nunjucks templates | ||
│ └── package.json // Ensures dependencies of source and build are in sync | ||
└── package.json // Workspaces configuration and top-level dependencies | ||
``` | ||
|
||
|
||
**Date:** 2023-04-21<!-- YYYY-MM-DD --> | ||
|
||
**Status:** accepted<!-- proposed, accepted, rejected, deprecated, superseded --> | ||
|
||
## Rationale | ||
|
||
<!-- The reason for the decision and any comments or disagreements cited. --> | ||
|
||
### Regrouping source and built package files within a shared package | ||
|
||
We may need to ship with dependencies in the future. For example: | ||
|
||
- peer dependencies to a polyfill library if we shipped a version compiled with `import` statements for the polyfills (rather than having the polyfills bundled in) | ||
- dependencies to other libraries, if we were shipping a Babel preset or PostCSS configuration | ||
|
||
Moving the source and built files within the same package helps us ensure these dependencies are resolved consistently. | ||
|
||
We don't have any dependencies yet, but moving the files would change the shape of our package and ask people to update their import paths for Nunjucks, making it a breaking change. Taking advantage of the breaking nature of the v5 release sets us up so we don't have to worry about future dependencies. | ||
|
||
This doesn't force us to ship the source files, as well, which we've decided to exclude (at least for now). Those can be excluded via an `.npmignore` files or the `files` field in `package.json`. | ||
|
||
### Removing built package files from version control | ||
|
||
The review app uses relative paths (`../../src/...`) to access the components of GOV.UK Frontend. While we cannot make it consume `govuk-frontend` the exact same way our users do (downloading from an `npm` registry), we can get pretty close using workspaces. Making it use `govuk-frontend` directly helps ensure that: | ||
- we're consuming the built files, not rebuilding from source with a process that we have to keep in sync with how the package is built | ||
- the actual files work as intended | ||
- `entries` in `package.json` are properly configured | ||
- dependencies get resolved as expected (we'll still need a different route to test `peerDependency` if we introduce them, though) | ||
|
||
For that to happen, we need the package rebuilt regularly rather than only at the point of release. If we were to leave it in the repository, there'd be a risk of committing built files that are in-between the previous release and the next one. Remove it it ensures we can rebuilt at will. | ||
|
||
### Updating our release process | ||
|
||
Including the built files in version control serves as a way to check that there's no weird code sneaking in. Changes to the package get highlighted during the review of the pull request raised to commit the new build of the package and the release. | ||
|
||
We'll need to keep checking for unexplained changes to the package as we release, even if it is by manually diffing in a first time (before further automation). | ||
|
||
### Leaving `dist` in its current location | ||
|
||
`dist` doesn't need to move to enable the previous points, and with the amount of changes in v5, we'll leave it out of the equation for now. | ||
|
||
## Risks and constraints | ||
|
||
<!-- Any accepted risks associated with the decision. --> | ||
|
||
- There may be some confusion with `package` previously being somewhere not to edit the files of and now becoming where the sources are. Similarly, we'll have two `dist` folders (at root and in `package`). Naming might need updating to sort that out. | ||
- External contributors may be used to the current structure of the repository and confused by the relocation. We can leave hints in README files (or maybe symlink) to the new locations to help with that. | ||
|
||
## Alternatives considered | ||
|
||
<!-- Other options discussed but decided against. --> | ||
|
||
### Regrouping source and built package files within a shared package | ||
|
||
- Not regrouping the source and built package within the same package now. This would mean waiting for the next breaking change to do it. We may need to have peer dependencies sooner than it. | ||
|
||
### Removing built package files from version control | ||
|
||
- We could also have built the package to a separate folder that's not in version control (for ex. `govuk-frontend-local-build`) to ensure we consume the built files without impacting `package`. We'd have to duplicate efforts to maintain two `package.json` files and lose testing our dependency resolution, package exports and complicate the linting and Dependabot updates. | ||
- Each PR could get a package deployed to the [Github npm registry](https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-npm-registry) allowing its installation by the review app down the line. This requires extra infrastructure though, and doesn't help when running locally. | ||
|
||
## Implications | ||
|
||
<!-- Impacts of this decision. --> | ||
|
||
- Users will need to update their import paths when migrating to v5, which we can document as part of the release notes/migration docs. | ||
- Ongoing new components (Exit This Page, Tasklist) will need to have their sources relocated before they can be merged in the new code structure. We can help with moving things and can release Exit This Page from a support branch if it needs to go before v5. | ||
- We need to keep making sure our configuration for the GOV.UK Prototype Kit keeps getting recognised by the Kit and picks up our files as necessary. | ||
|
||
## Contributors | ||
|
||
<!-- Who made this decision. --> | ||
- Colin Rotherham (@colinrotherham) | ||
- Oliver Byford (@36degrees) | ||
- Romaric Pascal (@romaricpascal) | ||
|
||
## Associated issues and pull requests (PRs) | ||
|
||
<!-- Add links to any related GitHub issues and / or PRs. --> | ||
https://github.com/alphagov/govuk-frontend/pull/3491 | ||
https://github.com/alphagov/govuk-frontend/pull/3498 | ||
|
||
## Outcomes | ||
|
||
<!-- Note any outcomes or consequences of the decision . --> | ||
|
||
Part of the discussion around this decision clarified that the review `app` does not need to take responsibility for all development processes. Performance monitoring, for example, can happily happen in its own workspaces and feed the app something to serve if necessary. Same could go for testing that the different compiled versions work appropriately it/when we start shipping more than one variant of compiled files. | ||
|
||
It was also mentioned, discussing the possibility of dependencies, that we'll need to be mindful of the implications of the dependencies we add (provenance, maintenance...). |