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

Update copy:files to glob “src” not “src/govuk” #2964

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

colinrotherham
Copy link
Contributor

This PR widens gulp copy:files to glob ./src/**/* not just the "govuk" directory

Why? Since #2851 was merged we're now running build tasks on files within:

./src/govuk
./src/govuk-prototype-kit

As suggested by @36degrees in #2851 (review) we could remove some unnecessary file operations:

(I think it'd be neater if the .js and .scss files were handled by the existing copy-files task, but that looks like it's caught up in the fact that config.src is actually src/govuk so happy to revisit that later)

These changes were made possible by:

@colinrotherham colinrotherham requested a review from a team November 4, 2022 15:16
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2964 November 4, 2022 15:17 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2964 November 7, 2022 13:35 Inactive
Base automatically changed from absolute-paths to main November 7, 2022 15:04
@colinrotherham
Copy link
Contributor Author

@36degrees Does this PR address your code review suggestion?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2964 November 8, 2022 12:57 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

👍🏻

@colinrotherham colinrotherham merged commit 4c3f799 into main Nov 8, 2022
@colinrotherham colinrotherham deleted the copy-from-outside-govuk branch November 8, 2022 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

3 participants