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

Mechanical refactor to group imports from common and govuk directories #281

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

dabreegster
Copy link
Contributor

Makes using these common libraries less verbose. https://javascript.info/import-export#re-export talks about this approach.

Copy link
Contributor

@Pete-Y-CS Pete-Y-CS left a comment

Choose a reason for hiding this comment

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

This PR looks fine but as I think about the task of putting the bid appraisal process into an app I wonder if we wanna put these gov uk tools into a separate svelte library which our other apps can import from?

@dabreegster
Copy link
Contributor Author

We will definitely want to share these! This is a step towards that at least; we could publish an NPM packge and then just swap "../govuk" with "govuk-svelte". Not exactly sure how to name that package yet... I wouldn't want to suggest to anyone external to ATE that it's ready to use. https://github.com/PaulioRandall/svelte-gds-design-system is the one related library I've found, also to consider using / taking inspiration from

@dabreegster
Copy link
Contributor Author

And if we want to punt on publishing to NPM, we can temporarily depend on code from here in another repo by using the GH link: https://github.com/dabreegster/osm2svelte/blob/8bad22ad7707a8dcfc79b3416acf8ef8c15c1b75/package.json#L30
But this really is a temporary hack

@dabreegster dabreegster merged commit 2a154ba into main Jul 21, 2023
2 checks passed
@dabreegster dabreegster deleted the combine_imports branch July 21, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants