Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build tasks: Clearly show subdirectory paths #3370

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Conversation

colinrotherham
Copy link
Contributor

@colinrotherham colinrotherham commented Mar 9, 2023

This PR helps to clearly show our build paths as part of:

Whilst pairing on #3318 it wasn't obvious which areas of the repo are being copied, written or built from config paths

For example, what's the difference between these three?

  1. configPaths.components
  2. join(configPaths.package, 'govuk/components')
  3. join(configPaths.package, 'govuk-esm/components')

I've ensured that only top-level paths are found in config/paths.js so their locations are clear:

Top level

join(paths.root, 'config')
join(paths.root, 'node_modules')

Source

join(paths.src, 'govuk/assets')
join(paths.src, 'govuk/components')

Review app

join(paths.app, 'views')
join(paths.app, 'views/examples')
join(paths.app, 'views/full-page-examples')
join(paths.app, 'views/layouts')

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 9, 2023 16:40 Inactive
@colinrotherham colinrotherham changed the title Clearly show subdirectory build paths Build tasks: Clearly show subdirectory paths Mar 9, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 9, 2023 16:59 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 9, 2023 19:43 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 10, 2023 16:40 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 13, 2023 14:01 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3370 March 17, 2023 11:23 Inactive
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes things a little more obvious 🙌🏻 Wondering if we'd need helpers to remove the many join(path.xyz,...) that we do into a path.xyz(...) but that can be done in a further PR 😄

@@ -20,7 +20,7 @@ export default async () => {
.filter(filterPath([`${componentName}/macro.njk`]))

return {
importFrom: slash(relative(configPaths.src, join(configPaths.components, macroPath))),
importFrom: slash(relative(paths.src, join(paths.src, 'govuk/components', macroPath))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question It's not your change, but isn't that equivalent to join('govuk/components', macroPath)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romaricpascal Spot on! Hidden away by configPaths.components until now

Will fix before merging, thanks again

This commit removes non-root paths from the paths config

It’s now easy to determine “source” versus “review app” directories

```js
join(paths.src, 'govuk/assets')
join(paths.src, 'govuk/components')
join(paths.app, 'views')
join(paths.app, 'views/layouts')
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants