infi-nl / the-infi-way

How we like to build software
https://way.infi.nl
Other
9 stars 8 forks source link

Added dev scripts for utility purposes #71

Closed yougotdirked closed 1 year ago

yougotdirked commented 1 year ago
netlify[bot] commented 1 year ago

Deploy Preview for the-infi-way ready!

Name Link
Latest commit 01b21eb75fe6d5030ea69d3503dbefb7e892fef1
Latest deploy log https://app.netlify.com/sites/the-infi-way/deploys/64abd160e7faff000832b673
Deploy Preview https://deploy-preview-71--the-infi-way.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jeroenheijmans commented 1 year ago

In general I'm quite sad we're straying further and further from the "super simple html only" setup we used to have. Personally I would prefer to find a way back to less or even zero dependencies. For perspective:

Which also indicates that this PR doesn't change much, relatively. Besides, I feel like a majority of people would prefer convenience scripts and packages, and @LucaScorpion you also give your go ahead. So I'm happy to go along with that 😊 - perhaps some time soon (if I'm not too late by that time) I'll craft a proposal for removing all dependencies once again going back to bare-bones. But until that time, let's go ahead with this approach?

EDIT: Danny rightfully alerted me that nowhere in the repository do we document that a "lean" repository is our aim, so there was also no convenient way for contributors to know about this approach. Something we should probably fix.


Having said that, I checked out this branch locally, ran npm ci and tried the documented npm start, but on Windows 10 it fails with this (version checks included for completeness):

[yougotdirked-dev-utility-scripts] D:\git\infi\the-infi-way>  npm --version
8.18.0
[yougotdirked-dev-utility-scripts] D:\git\infi\the-infi-way>  node --version
v18.16.0
[yougotdirked-dev-utility-scripts] D:\git\infi\the-infi-way>  npm start

> start
> concurrently 'npm run build:watch' 'npm run serve'

[0] ''npm' is not recognized as an internal or external command,
[0] operable program or batch file.
[1] 'run' is not recognized as an internal or external command,
[1] operable program or batch file.
[2] The filename, directory name, or volume label syntax is incorrect.
[3] ''npm' is not recognized as an internal or external command,
[3] operable program or batch file.
[2] build:watch' exited with code 1
[1] run exited with code 1
[0] 'npm exited with code 1
[4] 'run' is not recognized as an internal or external command,
[4] operable program or batch file.
[5] 'serve'' is not recognized as an internal or external command,
[5] operable program or batch file.
[5] serve' exited with code 1
[4] run exited with code 1
[3] 'npm exited with code 1

With npm run start I get exactly the same issue. Ironically, these kinds of gotcha's is exactly what I worry about when introducing new dependencies πŸ˜… (sorry, couldn't resist 😁). I tried this with both Powershell and cmd.

I did some initial investigation on why this is happening and whether it's my own machine giving troubles. I'll try to find a moment to double check this on my work laptop later on, and am also open to more input. I believe @yougotdirked you also use a Windows laptop, right? Or are you on macOS/Linux with bash?

jeroenheijmans commented 1 year ago

We fixed some final things together, @yougotdirked will check with a colleague if it now also works on *nix and then merge this PR. πŸŽ‰

LucaScorpion commented 1 year ago

Works nicely on Linux still!

To address some of your concerns @jeroenheijmans:

In general I'm quite sad we're straying further and further from the "super simple html only" setup we used to have. Personally I would prefer to find a way back to less or even zero dependencies.

I do fully agree with this. But currently (i.e. on main) the project doesn't actually run properly :grimacing: That is to say, the CSS doesn't load due to the way the path to that stylesheet is set up. Additionally, the steps in the contributing file didn't seem to work properly on Windows (or at least in Dirk's setup). While both of these are definitely things that we can fix without npm fluff, I think this having this is a fine temporaryβ„’ solution.

I feel like a majority of people would prefer convenience scripts and packages

Hmmm, I'm not sure. I feel like not having to install anything is also a big plus, but hard to say what the general consensus would be.

perhaps some time soon (if I'm not too late by that time) I'll craft a proposal for removing all dependencies once again going back to bare-bones

I was also considering this :grin: So I'd fully support this.

Either way, I do think merging this is still a good idea, since it straight up fixes the setup - even if we do intend to change it again later :)