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

add support for package versions #43

Closed
wants to merge 3 commits into from

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Nov 3, 2023

Description

this PR

  • replaces all instances of {{ VERSION }} in the module files with $"($package.name)@($package.version)"
  • adds a nupm version command to showcase how that works

when i install Nupm now, i have the following

> nupm version
nupm@0.1.0

@amtoine
Copy link
Member Author

amtoine commented Nov 3, 2023

@kubouch
do you thing we should add the number of commits above the last stable version?
e.g. [email protected]+123

that might require reading Git tags 👍

@kubouch
Copy link
Contributor

kubouch commented Nov 3, 2023

To me, replacing {{ VERSION }} does not seem necessary, you can simply read the value from package.nuon. Replacing {{ VERSION }} with the version number during install makes the modules unusable if they're not installed with nupm: They would just print "{{ VERSION }}", not the actual version. I'd rather avoid that.

@amtoine
Copy link
Member Author

amtoine commented Nov 3, 2023

so we need to install package.nuon next to the modules, right?

@kubouch
Copy link
Contributor

kubouch commented Nov 3, 2023

Ah, hmm, that won't work, we shouldn't install package.nuon. Then the package would need to keep track of it on its own. I really wouldn't do the {{ VERSION }} string replacement.

@amtoine
Copy link
Member Author

amtoine commented Nov 3, 2023

Ah, hmm, that won't work, we shouldn't install package.nuon. Then the package would need to keep track of it on its own. I really wouldn't do the {{ VERSION }} string replacement.

you mean nupm version should hardcode the version?

that wouldn't allow to have more fine-grained version, such as [email protected]+123, or you'd have to edit the "VERSION" every time there is a change 😕

@kubouch
Copy link
Contributor

kubouch commented Nov 3, 2023

Is hardcoding it too bad? With compiled languages like Rust, you could save it as a const into the binary (we do this in Nushell), but that's not applicable for dynamic Nushell code. I'd be curious how other languages do it. Do some languages do this string replacement thing?

If we don't like hard-coding, I'm thinking that it could be solved with profiles (previously I called them overlays in nuun). The profile could set up a metadata env var where you could fetch the versions of packages installed in the profile. But that seems even more complicated than the string replacement, although you wouldn't need to modify the module's code like you do in string replacement. Both string replacement and the profile env have the same disadvantage of being reliant on nupm. Without nupm, it would show wrong value and that's my main problem I have with this.

Also, the problem can be abstracted a bit: How to fetch package metadata from inside the package script/module? It might be useful for other things than versions.

@amtoine
Copy link
Member Author

amtoine commented Dec 11, 2023

this was an interesting attempt but, as you said @kubouch, the str replace trick is probably not what we want.

i'll close this for now and we'll try to focus on overlays and profiles in the future 😌

@amtoine amtoine closed this Dec 11, 2023
@amtoine amtoine deleted the package-version branch December 11, 2023 08:30
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