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

Update supported Crystal versions in "installation.md" #583

Merged
merged 2 commits into from
Sep 13, 2024

Conversation

SamantazFox
Copy link
Member

Thanks to two different users that made me realise that it was out of sync with our CI test matrix (version 1.8 and before fail due to the missing String.match!() function in the stdlib)

@syeopite
Copy link
Member

I was already planning on bumping the version with #577 :P

@SamantazFox
Copy link
Member Author

Oh, my bad, I didn't see your PR D:

@@ -110,7 +110,9 @@ docker-compose up

Follow the instructions for your distribution here: https://crystal-lang.org/install/

Note: Invidious currently supports the following Crystal versions: `1.9.2` / `1.8.2` / `1.7.X` / `1.6.X`
**Note:** Invidious currently supports the following Crystal versions: `1.9.2` / `1.10` / `1.11` / `1.12`. \
Copy link
Member

Choose a reason for hiding this comment

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

Considering iv-org/invidious#4256 should 1.9.X be supported?

Copy link
Member Author

@SamantazFox SamantazFox Aug 19, 2024

Choose a reason for hiding this comment

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

Ah, that's something I completely forgot about. Yeah, this was fixed in 1.10 (crystal-lang/crystal#13705). Do we add a warning for 1.9? Or just remove that version entirely?

Copy link
Member

Choose a reason for hiding this comment

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

I guess that depends on whether we consider "supporting" just meaning that Invidious can run on a particular version, or that it will run without any problems.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, true. I think we should go for the latter: "supported" means that there is no wildcards attached.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me. Can you remove 1.9?

Copy link
Member

@syeopite syeopite left a comment

Choose a reason for hiding this comment

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

Can you also include patch versions in the supported Crystal versions?

Eg: 1.10.X / 1.11.X / 1.12.X

@SamantazFox
Copy link
Member Author

@syeopite Sorry, I forgot to make that commit. Here are the requested changes!

Copy link
Member

@syeopite syeopite left a comment

Choose a reason for hiding this comment

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

Github does support admonitions now, and so does mkdocs. So maybe we should do something like this?

Warning

Note: Invidious currently supports the following Crystal versions: 1.10.x / 1.11.x / 1.12.x.

Versions 1.9.x and older are incompatible because we use features only present in the newer versions.

Versions 1.13.x should be compatible, however we did not test it.

@syeopite syeopite merged commit 4a3415a into master Sep 13, 2024
@syeopite syeopite deleted the crystal-version-update branch September 13, 2024 22:24
@ChunkyProgrammer
Copy link
Contributor

I dont think that the invidious ci needs to be ran for 1.9.2 if it"s not supported:
https://github.com/iv-org/invidious/blob/4782a6703819e0babfa4792892b691dd096eeac3/.github/workflows/ci.yml#L41

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.

3 participants