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

content: enhance README #1132

Merged
merged 3 commits into from
Dec 18, 2023
Merged

content: enhance README #1132

merged 3 commits into from
Dec 18, 2023

Conversation

Fupete
Copy link
Collaborator

@Fupete Fupete commented Dec 14, 2023

fix: #1072

@Fupete Fupete requested review from astagi and bfabio December 14, 2023 08:56
@Fupete Fupete self-assigned this Dec 14, 2023
Copy link
Contributor

github-actions bot commented Dec 14, 2023

⚡ Lighthouse report for the home page 🏠

Category Score
🟠 Performance 61
🟢 Accessibility 100
🟢 Best practices 100
🟢 SEO 100

See the full report...

Other pages

@bfabio
Copy link
Member

bfabio commented Dec 14, 2023

Uhm, we should IMO refactor the scripts a little bit, so that the prerequisites of the commands are explicit and self-documenting using pre-scripts.

That way the scripts section would also be leaner, more maintainable and more readable. I'd also rename prepare-* to avoid confusion, as prepare has a special meaning for npm

WDYT about:

   "scripts": {
     "copy-assets": "cp -r node_modules/bootstrap-italia/dist/svg/ static/ && cp -r node_modules/bootstrap-italia/dist/fonts static/",
     "...": "...",
     "prestart": "npm run copy-assets"
     "start": "gatsby develop",
     "...": "...",
     "prebuild": "npm run copy-assets"
     "build": "gatsby build --prefix-paths"

Also, I'd rename prepare-content (and the supporting scripts it runs under scripts/) to something more specific and self-documenting. AFAIR that script syncs the local examples with the latest (or tagged?) Bootstrap Italia components and it's supposed to run manually when a new version is out, is that right.

Something like sync-bootstrap-italia-examples or sync-design-system-examples/update-design-system-examples.

Bonus: it would make sense to run it from here automatically: https://github.com/italia/designers.italia.it/blob/main/.github/workflows/bsi-update.yml

@Fupete
Copy link
Collaborator Author

Fupete commented Dec 14, 2023

Uhm, we should IMO refactor the scripts a little bit, so that the prerequisites of the commands are explicit and self-documenting using pre-scripts.

That way the scripts section would also be leaner, more maintainable and more readable. I'd also rename prepare-* to avoid confusion, as prepare has a special meaning for npm

WDYT about:

   "scripts": {
     "copy-assets": "cp -r node_modules/bootstrap-italia/dist/svg/ static/ && cp -r node_modules/bootstrap-italia/dist/fonts static/",
     "...": "...",
     "prestart": "npm run copy-assets"
     "start": "gatsby develop",
     "...": "...",
     "prebuild": "npm run copy-assets"
     "build": "gatsby build --prefix-paths"

Also, I'd rename prepare-content (and the supporting scripts it runs under scripts/) to something more specific and self-documenting. AFAIR that script syncs the local examples with the latest (or tagged?) Bootstrap Italia components and it's supposed to run manually when a new version is out, is that right.

Something like sync-bootstrap-italia-examples or sync-design-system-examples/update-design-system-examples.

All cool to me. If these things are feasible in a short period of time let's proceed directly on this branch. If not, I would prefer to take an intermediate step by first updating the README to the state of the art, then later opening a PR dedicated to evolutions. WDYT?

Bonus: it would make sense to run it from here automatically: https://github.com/italia/designers.italia.it/blob/main/.github/workflows/bsi-update.yml

Not really. It is one thing to update BSI of the site. That can happen automatically, although at the moment PR for some reason actually still doesn't go through and will have to be fixed.
Other would be to auto-update the component examples in the design system documentation instead, copying from the BSI API, which impacts the need for a new release of the Design system documentation. For now I prefer to do by hand. Then later perhaps we can evaluate automatisms.

@bfabio
Copy link
Member

bfabio commented Dec 14, 2023

Also, I'd rename prepare-content (and the supporting scripts it runs under scripts/) to something more specific and self-documenting. AFAIR that script syncs the local examples with the latest (or tagged?) Bootstrap Italia components and it's supposed to run manually when a new version is out, is that right.
Something like sync-bootstrap-italia-examples or sync-design-system-examples/update-design-system-examples.

All cool to me. If these things are feasible in a short period of time let's proceed directly on this branch. If not, I would prefer to take an intermediate step by first updating the README to the state of the art, then later opening a PR dedicated to evolutions. WDYT?

Sounds good, the renaming part is really simple and can be done quickly in this PR.

Bonus: it would make sense to run it from here automatically: https://github.com/italia/designers.italia.it/blob/main/.github/workflows/bsi-update.yml

Not really. It is one thing to update BSI of the site. That can happen automatically, although at the moment PR for some reason actually still doesn't go through and will have to be fixed. Other would be to auto-update the component examples in the design system documentation instead, copying from the BSI API, which impacts the need for a new release of the Design system documentation. For now I prefer to do by hand. Then later perhaps we can evaluate automatisms.

Uhm, so there is no link between the locally installed Bootstrap Italia and the examples generation. This threw me off: https://github.com/italia/designers.italia.it/blob/main/scripts/generate-examples.js#L6 and I think it's wrong, IIUC.

@Fupete Fupete merged commit 284c877 into main Dec 18, 2023
4 checks passed
@Fupete Fupete deleted the content-enhance-readme-202312 branch December 18, 2023 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhance "Quick start" in README.md
3 participants