Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Display Gleam version number on playground navbar #10

Merged
merged 7 commits into from
Dec 9, 2024

Conversation

brettkolodny
Copy link
Contributor

@brettkolodny brettkolodny commented Nov 21, 2024

After wanting to try out the new Gleam version but being unsure whether or not the playground had been updated yet I thought it would be a good idea to include Gleam version being used somewhere within the UI. If there is a more clear place to put it please let me know and I can adjust the PR accordingly.

Additionally I updated the code to read the gleam version from the command line both for downloading the correct version of the Gleam compiler and for displaying the version on the frontend. This way the two are never out of sync, but it does mean that the variable will needed to be added to GitHub.

I also had to change the target to Erlang from JavaScript. As far as I can tell this should be fine since it seems like the action's environment is already pulling in Erlang.

CleanShot 2024-11-21 at 13 03 44@2x


As a bit of an aside, I was wondering if there is any use in letting users somehow choose which version of the Gleam compiler is being used for the playground. If that seems like a worthwhile feature I'd be happy to open up an issue for further discussion, and take on development as well.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

I dig it! Could you make it so gleam run and the compiler downloader script work without setting environment variables please. It doesn't vary per environment so it should be derived from the code, not the environment.

Ideally the CI fails if its version doesn't match the version they use, but I'm happy for that not to be part of your contribution if you prefer.

@brettkolodny
Copy link
Contributor Author

So I updated the code to read from a GLEAM_VERSION file instead. Both the code and the download script read from this file and the code will now throw an error if the download was unsuccessful (and thus probably the incorrect version number).

Not 100% sure if this is what you want or not. If it's not I can take all of the fancy code for reading the version number out and just leave the hard-coded one in and add a line to the README to update it in that spot

Comment on lines +262 to +266
fn read_gleam_version() -> snag.Result(String) {
gleam_version
|> simplifile.read()
|> file_error("Failed to read glema version at path " <> gleam_version)
}
Copy link

Choose a reason for hiding this comment

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

Note that at runtime you may be able to use this as well build/dev/javascript/gleam_version.

Copy link

@Beanow Beanow Dec 1, 2024

Choose a reason for hiding this comment

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

Actually nevermind, that would be what gleam version ran on the host to do the precompiling.
The compiler version itself would be in playground/wasm-compiler/package.json

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you! I think you've removed the target in gleam.tom by mistake though

@brettkolodny
Copy link
Contributor Author

Oops sorry, just re-added!

@brettkolodny brettkolodny requested a review from lpil December 9, 2024 14:14
Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you!!

@lpil lpil merged commit 7fb248f into gleam-lang:main Dec 9, 2024
1 check passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants