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

fix: remove yarn, use npm #5

Merged
merged 3 commits into from
Dec 1, 2023
Merged

Conversation

robertsLando
Copy link
Collaborator

No description provided.

@robertsLando robertsLando changed the title Main fix: remove yarn, use npm Nov 30, 2023
@robertsLando
Copy link
Collaborator Author

@jmgiaever I have added some FIXME comment on places I'm not sure what to use

@jmgiaever
Copy link
Contributor

Ty mate 🫶 Hah, I last night fixed stuff for 9.4.x 😆 I will try to transition for npm tomorrow. It's just burning around me atm. High season at work.

Why the change from yarn btw?

@robertsLando
Copy link
Collaborator Author

robertsLando commented Nov 30, 2023

Hi Joachim 🫶

I saw you pushed some stuff recently, feel free to edit the missing pieces here if needed (or tell me what to do, I don't know anything about snap), I would drop the layout part entirely but I'm not sure so I let you check it when you have time, just wanted to help with the transition to make it less painfull as I know you are very busy in this period :)

Why the change from yarn btw?

Long story short is it turned me crazy with bugs on docker, the only way I found to make everything work was to use some mixed npm commands, after making it work I just decided to drop yarn entirely as there was no reason to have both, now image size has been reduced a lot and it also take only 7 minutes to build on all archs compared to 1h 🎉

You can find the full story behind this here: zwave-js/zwave-js-ui#3430 (comment)

Copy link
Contributor

@jmgiaever jmgiaever left a comment

Choose a reason for hiding this comment

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

Note to myself: check prime on line 178, which excludes npm to be bundled with the snap. This is probably needed now.

@robertsLando is npx needed?

snap/snapcraft.yaml Outdated Show resolved Hide resolved
@@ -95,7 +95,7 @@ apps:
restart:
command: bin/restart

layout:
layout: # FIXME: npm cache is on `~/.npm`
Copy link
Contributor

Choose a reason for hiding this comment

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

Means it's a subdirectory of the program files? I might have to do some tricks here, as that might be a Read Only partition.

Do you know if it's possible to specify the cache dir?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

npm config set cache /pat/to/cache or export npm_config_cache=/path/to/cache

Copy link
Contributor

@jmgiaever jmgiaever left a comment

Choose a reason for hiding this comment

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

Note to self: is jq and yq needed anymore, elsewhere in the project?

@robertsLando
Copy link
Collaborator Author

robertsLando commented Nov 30, 2023

jq is no longer needed, it was to remove yarn dev deps. Dunno about yq it's not needed on my side 🤷🏼‍♂️

@jmgiaever
Copy link
Contributor

I might use them to parse the settings file back and fourth, in the Bourne scripts. Can't recall. I think it's time to rework the project as so much has changed the past years.

@jmgiaever
Copy link
Contributor

jmgiaever commented Dec 1, 2023

@robertsLando is npx needed?

And what about corepack?

@robertsLando
Copy link
Collaborator Author

Never used, dunno what is that for?

@jmgiaever jmgiaever merged commit cd14c93 into giaever-online-iot:main Dec 1, 2023
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